-
Notifications
You must be signed in to change notification settings - Fork 71
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.
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
make pulumi-version-file also work in non-install mode #1218
Conversation
The pulumi-version-file option was introduced in #1204. However it was only introduced for installation mode, not if a pulumi command is given. The latter has separate config parsing code, so let's introduce the same option there as well.
016ff26
to
781cd44
Compare
/cc @corymhall who probably has most context here. I'd appreciate if you could have a quick look! |
src/config.ts
Outdated
@@ -24,6 +24,9 @@ export type InstallationConfig = rt.Static<typeof installationConfig>; | |||
|
|||
export function makeInstallationConfig(): rt.Result<InstallationConfig> { | |||
let pulumiVersion = getInput('pulumi-version'); | |||
if (pulumiVersion === "undefined") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When will this be the string "undefined"
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an awkward side effect of the way we're testing this. When we're setting up the mocked config here, the "undefined" gets turned into a string. Though maybe we should be using mocks instead for at least the version so we can avoid this 🤔
This PR has been shipped in release v5.3.3. |
The pulumi-version-file option was introduced in #1204. However it was only introduced for installation mode, not if a pulumi command is given. The latter has separate config parsing code, so let's introduce the same option there as well.
Fixes: #1215