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.

Sign up for GitHub

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

ci: fix publish step check before tagging #39

Merged
merged 1 commit into from
Jan 16, 2025
Merged

ci: fix publish step check before tagging #39

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/workflows/release.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,11 @@ jobs:
- uses: katyo/publish-crates@v2
with:
registry-token: ${{ secrets.CARGO_REGISTRY_TOKEN }}
id: publish-crates
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you get a chance to test if this line alone fixes the error? Running actionlint on the main branch flags the missing id for this GitHub Actions step, as it’s used later but isn’t currently defined.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i did try to check it, but the output was always an empty string, so i couldn't be sure if it would've worked or not with an actual release.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test run here shows fromJSON works when the output includes the array string for a new version. However, if there’s no new version, the output is an empty string, causing fromJSON to fail.

The outcome == 'success' check should work well, but an empty string check could also suffice.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think i'll opt to leave it with `outcome == 'success' since it's a bit easier to understand without looking through the action's docs for someone else on our team to work on this in the future. but good to see the output on a successful run as well

if: ${{ steps.release.outputs.release_created }}
# https://github.com/googleapis/release-please-action?tab=readme-ov-file#creating-majorminor-tags
- name: Tag versions
if: ${{ steps.release.outputs.release_created && fromJSON(steps.publish-crates.outputs.published).* }}
if: ${{ steps.release.outputs.release_created && steps.publish-crates.outcome == 'success' }}
Copy link
Contributor Author

@ctran88 ctran88 Jan 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the stringified version of an empty array in JS is just an empty string (not counting however @actions/core decides to serialize the value).

given error message from the last successful release, it made it seem like the value was an empty string (Path '', line 0, position 0.) so i wasn't too confident in just checking that.

a failure on the publish step should result in a step failure based on https://github.com/katyo/publish-crates/blob/5e67639f17e8a1f221e804c2bd47f7a253e45dac/src/main.ts#L145

so i figured we could just check for the success of the publish step instead.

run: |
git config user.name github-actions[bot]
git config user.email 41898282+github-actions[bot]@users.noreply.github.com
Expand Down
Loading