-
Notifications
You must be signed in to change notification settings - Fork 1
189 refactor new code imported from gl workflow scripts #232
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
189 refactor new code imported from gl workflow scripts #232
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #232 +/- ##
=======================================
Coverage 90.24% 90.24%
=======================================
Files 42 42
Lines 2009 2009
=======================================
Hits 1813 1813
Misses 196 196 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| s3_artifacts = S3Artifacts(s3_bucket_name) | ||
|
|
||
| commitish_short = commitish[:8] |
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.
The gardenlinux.features.CName class is capable of calculating the correct short commit hash based on the Git commit hash and defined behavior. Please do not truncate commit hashes elsewhere in the code.
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.
This particular short commit variable is used to construct an s3 objects filter. Not sure how CName instance could help there.
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.
Otherwise replaced its use for CName call below with full commit id.
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.
Use commitish_short = cname.commit_id for that purpose or directly Prefix=f"meta/singles/{cname}-{version}-{cname.commit_id}".
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.
You may also use Prefix=f"meta/singles/{cname.cname} if applicable.
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.
Switched to using cname.cname where appropriate. cname.commit_id however doesn't work as expected and breaks tests. Will check it out in a separate PR.
NotTheEvilOne
left a 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.
LGTM
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #189
This PR also includes a fixed release notes mockup file.