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.

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rowanmanning
Copy link

@rowanmanning rowanmanning commented Dec 19, 2023

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:

- 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:

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.

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.

Approach

I've moved the releases_created output back inside the conditional to switch the behaviour to match v3.

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 to true or false. It might be worth changing this documentation to explain that the values are all strings rather than booleans.

Copy link

google-cla bot commented Dec 19, 2023

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.

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`.
@psYifanChen
Copy link

@chingor13 @bcoe
Could anyone review this PR please?

@toddbaert
Copy link

@chingor13 is there any hope of a release with this fix?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants