Skip to content
This repository has been archived by the owner on Aug 29, 2023. It is now read-only.

fix initial run of release-check workflow #213

Merged
merged 2 commits into from
Oct 18, 2021

Conversation

marten-seemann
Copy link
Contributor

@marten-seemann marten-seemann commented Oct 17, 2021

When we first deploy this workflow, we're adding the version.json file. For this PR, we don't want to run the release-check workflow, as a release for this version number already exists.

Tested on the testing branch. Since this is a called workflow, we don't have run a new deployment to roll out this fix :)

- name: Install semver (node command line tool)
if: env.INITIAL_RUN == 'false'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's really annoying that there's no way to exit early from a GH Actions workflow (without failing the workflow run).

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if you could split this workflow into two jobs; have the second job depend on the first one, and only run if INITIAL_RUN==true.

Copy link
Contributor

Choose a reason for hiding this comment

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

To some degree, we could consider the same for COMPARE_TO.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, I think in a previous PR I had suggested to use underscores to make env var names like COMPARETO easier to read. Maybe that slipped through the cracks :)

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 wonder if you could split this workflow into two jobs; have the second job depend on the first one, and only run if INITIAL_RUN==true.

That comes at the expense of booting up two runners, checking out the repo twice and passing around env.VERSION. I still hope that GitHub gives us a way to cleanly do an early exit from workflows :)

Hmm, I think in a previous PR I had suggested to use underscores to make env var names like COMPARETO easier to read. Maybe that slipped through the cracks :)

I remember making that change, but it must have gotten swallowed by git at some point. Sorry.

@marten-seemann marten-seemann force-pushed the fix-initial-release-check branch 3 times, most recently from 67207ed to 9a68a6c Compare October 18, 2021 11:21
@marten-seemann
Copy link
Contributor Author

The logic in the original PR was wrong. In order to determine if this is the initial run of a workflow, we can't only look if version.json was added. That's a required condition, but not sufficient:
Imagine that we're cutting a new release on a release branch that doesn't yet have a version.json. In order to cut the release, we'd create the version.json with the new version number. We still should run the release-check workflow in this case.

I fixed the logic by checking that in addition to version.json being added we also use a version number for which a git tag already exists.

- name: Install semver (node command line tool)
if: env.INITIAL_RUN == 'false'
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if you could split this workflow into two jobs; have the second job depend on the first one, and only run if INITIAL_RUN==true.

- name: Install semver (node command line tool)
if: env.INITIAL_RUN == 'false'
Copy link
Contributor

Choose a reason for hiding this comment

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

To some degree, we could consider the same for COMPARE_TO.

- name: Install semver (node command line tool)
if: env.INITIAL_RUN == 'false'
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, I think in a previous PR I had suggested to use underscores to make env var names like COMPARETO easier to read. Maybe that slipped through the cracks :)

@marten-seemann marten-seemann merged commit e97c8f2 into master Oct 18, 2021
@marten-seemann marten-seemann deleted the fix-initial-release-check branch October 18, 2021 12:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants