-
Notifications
You must be signed in to change notification settings - Fork 363
chore(ci): clean up release and promote workflows #10349
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
Conversation
Preview: https://patternfly-react-pr-10349.surge.sh A11y report: https://patternfly-react-pr-10349-a11y.surge.sh |
# Fetches all tags from 'origin' and updates local tags, only fetches the latest commit. | ||
- name: Fetch tags | ||
run: git fetch --depth=1 origin +refs/tags/*:refs/tags/* | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've remove this as fetch-depth: 0
already fetches all the tags, see actions/checkout#217 (comment).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fetch depth 1 was added (i believe) to make sure we weren't republishing EVERY package in the monorepo each time a single change went into one package. It only republished the relevant packages. I guess we can watch to make sure this doesnt undo that effort now that it's removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So from what I understand Lerna should take care of this for us, but I can see this issue happening if Lerna does not detect any tags. That perhaps happened before if the fetch-depth
wasn't set in the action, in which case the default is 1
, which would only fetch the latest commit, and perhaps not all the tags.
The Lerna docs link to an Nx example that seem to operate on the same premise. From what I can gather from community blog posts it seems that fetch-depth: 1
should be enough for Lerna to do the right thing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since fetch-depth has 1 as a default value, this should be okay. let's just keep an eye on it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
.github/workflows/promote.yml
Outdated
core-version: | ||
description: The PatternFly core version | ||
release-version: | ||
description: The version of PatternFly to promote to. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, the argument in this workflow is the @patternfly/patternfly package version which is updated where ever necessary in the various packages in the patternfly-react monorepo. So it should be the same as the promoted version, but it isn't necessarily always.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understood that as well, but I thought core-version
was a bit non-descriptive, I've changed this to patternfly-version
now, with some additional description.
Signed-off-by: Jon Koops <[email protected]>
Cleans up the release and promote workflows so that secrets are only being passed where used, as well as removing some redundant code.