-
Notifications
You must be signed in to change notification settings - Fork 25
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
fix: Update makeLatest condition to exclude alpha and beta branches #209
Conversation
Signed-off-by: Ovler <cjin20@syr.edu>
Reviewer's Guide by SourceryThis pull request updates the State diagram for release artifact tagging logicstateDiagram-v2
[*] --> CheckBranchName
CheckBranchName --> ContainsAlphaOrBeta: Check ref_name
ContainsAlphaOrBeta --> NotLatest: Yes
ContainsAlphaOrBeta --> MarkAsLatest: No
NotLatest --> [*]: makeLatest=false
MarkAsLatest --> [*]: makeLatest=true
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @Ovler-Young - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
@@ -75,7 +75,7 @@ jobs: | |||
allowUpdates: true | |||
replacesArtifacts: false | |||
omitBodyDuringUpdate: true | |||
makeLatest: true | |||
makeLatest: ${{ !(contains(github.ref_name, 'alpha') || contains(github.ref_name, 'beta')) }} |
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.
suggestion: Consider extracting the repeated release type check into a reusable expression
This expression is repeated 7 times in the workflow. Consider defining it as a workflow-level environment variable or composite action to improve maintainability. For example: env: { IS_STABLE_RELEASE: ${{ !(contains(github.ref_name, 'alpha') || contains(github.ref_name, 'beta')) }} }
Suggested implementation:
allowUpdates: true
replacesArtifacts: false
omitBodyDuringUpdate: true
makeLatest: ${{ env.IS_STABLE_RELEASE }}
allowUpdates: true
replacesArtifacts: false
omitBodyDuringUpdate: true
makeLatest: ${{ env.IS_STABLE_RELEASE }}
You'll need to add this environment variable definition at the workflow level (near the top of the file, likely just after the name:
field):
env:
IS_STABLE_RELEASE: ${{ !(contains(github.ref_name, 'alpha') || contains(github.ref_name, 'beta')) }}
Also, you'll need to replace all other occurrences of this same expression throughout the workflow file with ${{ env.IS_STABLE_RELEASE }}
. Based on your comment, there are 7 total occurrences that need to be replaced.
Summary by Sourcery
CI: