Skip to content

Updated SDK docs on output - #313

Open
pierskarsenbarg wants to merge 1 commit into
mainfrom
apply-docs
Open

Updated SDK docs on output#313
pierskarsenbarg wants to merge 1 commit into
mainfrom
apply-docs

Conversation

@pierskarsenbarg

Copy link
Copy Markdown
Member

The previous version was technically correct in that you shouldn't create resources within an apply function but was incorrect that it is not allowed

The previous version was technically correct in that you shouldn't create resources within an apply function but was incorrect that it is not allowed
@pierskarsenbarg
pierskarsenbarg requested a review from a team as a code owner August 6, 2024 10:17
@pierskarsenbarg pierskarsenbarg added the impact/no-changelog-required This issue doesn't require a CHANGELOG update label Aug 6, 2024
@pierskarsenbarg

Copy link
Copy Markdown
Member Author

See also pulumi/pulumi#16885

@tgummerer tgummerer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry about the delay reviewing this.

Comment thread sdk/Pulumi/Core/Output.cs
/// can be properly tracked.
/// <para/>
/// <paramref name="func"/> is not allowed to make resources.
/// <paramref name="func"/> should not be used to create resources unless necessary. Creating resources within 'func' can cause unknown behaviour.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// <paramref name="func"/> should not be used to create resources unless necessary. Creating resources within 'func' can cause unknown behaviour.
/// <paramref name="func"/> should not be used to create resources unless necessary as `func` may not be run during some program executions.

Let's make it the same as in pulumi/pulumi#16885.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

impact/no-changelog-required This issue doesn't require a CHANGELOG update

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants