Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow environment variables to have a null value #6520

Merged
merged 2 commits into from
Apr 1, 2021
Merged

Allow environment variables to have a null value #6520

merged 2 commits into from
Apr 1, 2021

Conversation

MisinformedDNA
Copy link
Contributor

Fixes #6499

@github-actions
Copy link

PR is now waiting for a maintainer to run the acceptance tests.

Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

1 similar comment
@github-actions
Copy link

PR is now waiting for a maintainer to run the acceptance tests.

Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

@MisinformedDNA
Copy link
Contributor Author

@justinvp @mikhailshilkov

Do you guys want any changes? Should I note this in the CHANGELOG?

@mikhailshilkov
Copy link
Member

LGTM overall

@komalali Do we want to do the same across all languages? E.g. TypeScript doesn't mark it as nullable either:

pulumi/sdk/nodejs/x/automation/localWorkspace.ts

Line 614 in f9579b0

envVars?: { [key: string]: string };

@komalali
Copy link
Member

LGTM. We do want this across languages but that shouldn't have to be @MisinformedDNA's cross to bear. An addition to the CHANGELOG will suffice 🙏🏽

@komalali
Copy link
Member

/run-acceptance-tests

@github-actions
Copy link

Please view the results of the PR Build + Acceptance Tests Run Here

1 similar comment
@github-actions
Copy link

Please view the results of the PR Build + Acceptance Tests Run Here

mikhailshilkov
mikhailshilkov approved these changes Mar 30, 2021
View reviewed changes
Copy link
Member

@mikhailshilkov mikhailshilkov left a comment

Choose a reason for hiding this comment

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

LGTM. Could you please add a changelog entry and we'll merge. Thank you!

@github-actions
Copy link

PR is now waiting for a maintainer to run the acceptance tests.

Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

@mikhailshilkov
Copy link
Member

@MisinformedDNA Sorry to bother you again but there are conflicts in two files now. Could you please resolve them?

@MisinformedDNA
Allow environment variables to have a null value
ac1cc78 
# Conflicts:
#	sdk/dotnet/Pulumi.Automation/Commands/IPulumiCmd.cs
#	sdk/dotnet/Pulumi.Automation/Commands/LocalPulumiCmd.cs
@MisinformedDNA
Update CHANGELOG_PENDING
91b5648
@github-actions
Copy link

github-actions bot commented Apr 1, 2021

PR is now waiting for a maintainer to run the acceptance tests.

Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

@komalali komalali mentioned this pull request Apr 1, 2021
Open
@komalali komalali merged commit 17d1ce5 into pulumi:master Apr 1, 2021
komalali added a commit that referenced this pull request Apr 1, 2021
@komalali
Revert "Allow environment variables to have a null value (#6520)"
ee50e03 
This reverts commit 17d1ce5.
@komalali komalali mentioned this pull request Apr 1, 2021
Merged
komalali added a commit that referenced this pull request Apr 1, 2021
@komalali
Revert "Allow environment variables to have a null value (#6520)" (#6677
7f8985a 
)

This reverts commit 17d1ce5.
@mikhailshilkov mikhailshilkov mentioned this pull request Apr 1, 2021
Closed
abhinav pushed a commit to pulumi/pulumi-dotnet that referenced this pull request Jan 11, 2023
@MisinformedDNA
Allow environment variables to have a null value (pulumi/pulumi#6520)
efb017d
abhinav pushed a commit to pulumi/pulumi-dotnet that referenced this pull request Jan 11, 2023
@komalali
Revert "Allow environment variables to have a null value (pulumi/pulu…
dfff2f3 
…mi#6520)" (pulumi/pulumi#6677)

This reverts commit efb017d.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mikhailshilkov mikhailshilkov mikhailshilkov approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

LocalWorkspaceOptions.EnvironmentVariables should allow null values
3 participants
@MisinformedDNA @mikhailshilkov @komalali