-
Notifications
You must be signed in to change notification settings - Fork 206
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
fix: undo breaking change in releases_created output #915
base: main
Are you sure you want to change the base?
fix: undo breaking change in releases_created output #915
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
4132aea
to
c68257e
Compare
@chingor13 @bcoe |
@chingor13 is there any hope of a release with this fix? |
@chingor13 is there any hope of this being merged? |
An undocumented breaking change slipped into the v4 release which causes the conditional in actions to stop working as documented. In v3 the following step would only run if no releases were created. In v4 the action will still run: ```yml - run: echo "releases happened" if: ${{ steps.release.outputs.releases_created }} ``` This is because the @actions/core npm package `core.setOutput` method always casts values to a string, so when we call the following: ```js core.setOutput('releases_created', releases.length > 0); ``` The output of the action will always either be `"true"` or `"false"` as a string. This is a problem because our `if` statement above will always evaluate this to `true` and run the conditional step. The only reason this worked in v3 is because the `core.setOutput` was called only if `releases.length` was greater than zero rather than being passed the condition. * [Relevant v3 code](https://github.com/google-github-actions/release-please-action/blob/db8f2c60ee802b3748b512940dde88eabd7b7e01/index.js#L223-L224) * [Equivalent v4 code](https://github.com/google-github-actions/release-please-action/blob/cc61a07e2da466bebbc19b3a7dd01d6aecb20d1e/src/index.ts#L165) So in v3 the possible values for `releases_created` was actually `"true"` or `undefined`. This was never an issue because `"true"` is truthy and `undefined` is falsy so the conditions in our actions worked as expected. I've moved the `releases_created` output back inside the conditional to switch the behaviour to match `v3`.
c68257e
to
845a6f3
Compare
The issue this fixes
An undocumented breaking change slipped into the v4 release which causes the conditional in actions to stop working as documented. I encountered it when migrating some of my projects and an issue has already been opened (#912).
In v3 the following step would only run if releases were created. In v4 the action will always run:
This is because the
@actions/core
npm packagecore.setOutput
method always casts values to a string, so when we call the following:The output of the action will always either be
"true"
or"false"
as a string. This is a problem because ourif
statement above will always evaluate this totrue
and run the conditional step.The only reason this worked in v3 is because the
core.setOutput
was called only ifreleases.length
was greater than zero rather than being passed the condition.Relevant v3 code
Equivalent v4 code
So in v3 the possible values for
releases_created
was actually"true"
orundefined
. This was never an issue because"true"
is truthy andundefined
is falsy so the conditions in our actions worked as expected.Approach
I've moved the
releases_created
output back inside the conditional to switch the behaviour to matchv3
.Alternatives
If you're not happy with this change then I think the documentation should be updated to highlight this gotcha and the changelog for v4 should include it.
Potential additional work
core.setOutput
casting to a string has been noted in issues in the@actions/core
repo before, but all the documentation on this action indicates that outputs are set totrue
orfalse
. It might be worth changing this documentation to explain that the values are all strings rather than booleans.