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

Promote flag default doesn't agree with docs #361

Closed
joeyparrish opened this issue Apr 29, 2024 · 4 comments · Fixed by #362
Closed

Promote flag default doesn't agree with docs #361

joeyparrish opened this issue Apr 29, 2024 · 4 comments · Fixed by #362
Labels
bug Something isn't working

Comments

@joeyparrish
Copy link

TL;DR

The promote flag is documented as defaulting to true, but actually defaults to false.

Expected behavior

In v0, it was defaulted to true with:

    const promote = getInput('promote');
    // ...
    if (promote === '' || String(promote).toLowerCase() === 'true') {
      appDeployCmd.push('--promote');
    } else {
      appDeployCmd.push('--no-promote');
    }

Observed behavior

In v2, there is no explicit default anywhere, so the parseBoolean default of false takes over.

    const promote = parseBoolean(getInput('promote'));
    // ...
    if (promote) {
      appDeployCmd.push('--promote');
    } else {
      appDeployCmd.push('--no-promote');
    }

Action YAML

# N/A AFAICT

Log output

# N/A AFAICT

Additional information

If the docs are correct, I believe the fix could be as simple as changing:

    const promote = parseBoolean(getInput('promote'));

to:

    const promote = parseBoolean(getInput('promote'), /* default= */ true);

If the docs are wrong, then the obvious fix is to correct the docs.

@joeyparrish joeyparrish added the bug Something isn't working label Apr 29, 2024
joeyparrish added a commit to joeyparrish/shaka-player that referenced this issue Apr 29, 2024
Since upgrading the deployment action from v0 to v2, and stumbling
into google-github-actions/deploy-appengine#361, new versions were not
properly promoted.  However, our promotion setting in the workflow was
actually ignored the whole time due to a missing "echo".

Closes shaka-project#6502
joeyparrish added a commit to shaka-project/shaka-player that referenced this issue Apr 29, 2024
Since upgrading the deployment action from v0 to v2, and stumbling into
google-github-actions/deploy-appengine#361, new versions were not
properly promoted. However, our promotion setting in the workflow was
actually ignored the whole time due to a missing "echo".

Closes #6502
joeyparrish added a commit to shaka-project/shaka-player that referenced this issue Apr 29, 2024
Since upgrading the deployment action from v0 to v2, and stumbling into
google-github-actions/deploy-appengine#361, new versions were not
properly promoted. However, our promotion setting in the workflow was
actually ignored the whole time due to a missing "echo".

Closes #6502
@sethvargo
Copy link
Member

The default value in the action.yml is true. Are you seeing a different behavior?

@joeyparrish
Copy link
Author

joeyparrish commented Apr 29, 2024

Yes. I was passing a value that happened to be blank due to a dumb workflow bug.

    with:
      promote: ${{ env.APPSPOT_PROMOTE }}

That variable was meant to be "true" or "false", but it was left out of the environment entirely. It would appear that the default in action.yml was not used in this case.

This didn't become obvious until I upgraded from v0 to v2, since v0 had the default implemented explicitly in TypeScript.

sethvargo added a commit that referenced this issue Apr 29, 2024
This restores a v0 and v1 behavior wherein the empty string is considered "true" instead of "false". Fixes GH-361.
@sethvargo
Copy link
Member

Right - if you pass in a value, it will use the empty string (whereas not providing a value uses the default).

sethvargo added a commit that referenced this issue Apr 29, 2024
This restores a v0 and v1 behavior wherein the empty string is
considered "true" instead of "false". Fixes GH-361.
@joeyparrish
Copy link
Author

Thanks!

joeyparrish added a commit to shaka-project/shaka-player that referenced this issue May 7, 2024
Since upgrading the deployment action from v0 to v2, and stumbling into
google-github-actions/deploy-appengine#361, new versions were not
properly promoted. However, our promotion setting in the workflow was
actually ignored the whole time due to a missing "echo".

Closes #6502
joeyparrish added a commit to shaka-project/shaka-player that referenced this issue May 7, 2024
Since upgrading the deployment action from v0 to v2, and stumbling into
google-github-actions/deploy-appengine#361, new versions were not
properly promoted. However, our promotion setting in the workflow was
actually ignored the whole time due to a missing "echo".

Closes #6502
joeyparrish added a commit to shaka-project/shaka-player that referenced this issue May 7, 2024
Since upgrading the deployment action from v0 to v2, and stumbling into
google-github-actions/deploy-appengine#361, new versions were not
properly promoted. However, our promotion setting in the workflow was
actually ignored the whole time due to a missing "echo".

Closes #6502
joeyparrish added a commit to shaka-project/shaka-player that referenced this issue May 7, 2024
Since upgrading the deployment action from v0 to v2, and stumbling into
google-github-actions/deploy-appengine#361, new versions were not
properly promoted. However, our promotion setting in the workflow was
actually ignored the whole time due to a missing "echo".

Closes #6502
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging a pull request may close this issue.

2 participants