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

chore(ci): clean up release and promote workflows #10349

Merged
merged 1 commit into from
May 13, 2024

Conversation

jonkoops
Copy link
Contributor

@jonkoops jonkoops commented May 8, 2024

Cleans up the release and promote workflows so that secrets are only being passed where used, as well as removing some redundant code.

@patternfly-build
Copy link
Contributor

patternfly-build commented May 8, 2024

Comment on lines -33 to -36
# 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/*

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've remove this as fetch-depth: 0 already fetches all the tags, see actions/checkout#217 (comment).

Copy link
Contributor

@nicolethoen nicolethoen May 10, 2024

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.

Copy link
Contributor Author

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.

Copy link
Contributor

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.

Copy link
Contributor

@dlabaj dlabaj left a comment

Choose a reason for hiding this comment

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

LGTM

@jonkoops jonkoops changed the title fix(ci): fix release pipeline fix(ci): clean up release and promote workflows May 10, 2024
@jonkoops jonkoops changed the title fix(ci): clean up release and promote workflows chore(ci): clean up release and promote workflows May 10, 2024
@jonkoops jonkoops marked this pull request as ready for review May 10, 2024 14:13
core-version:
description: The PatternFly core version
release-version:
description: The version of PatternFly to promote to.
Copy link
Contributor

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.

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 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 <jonkoops@gmail.com>
@nicolethoen nicolethoen merged commit 76c4e2f into patternfly:main May 13, 2024
13 checks passed
@jonkoops jonkoops deleted the fix-release branch May 13, 2024 19:28
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.

4 participants