Closed rowanmanning closed 1 week ago
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.
@chingor13 @bcoe Could anyone review this PR please?
@chingor13 is there any hope of a release with this fix?
@chingor13 is there any hope of this being merged?
Any movement on this at all? Or has this been broken so long now that fixing it will break just as many workflows as this PR fixes?
I'm not sure if the maintainers are aware of this bug, we've had no response for almost a year and I think this PR probably can't be merged now – too many people have switched to the new behaviour.
I'm going to close this PR and have commented again on the issue to say that ideally this would be documented as a breaking change. Not gonna hold my breath though.
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.