-
Notifications
You must be signed in to change notification settings - Fork 97
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
Improve release workflow #2438
base: release/current
Are you sure you want to change the base?
Improve release workflow #2438
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## release/current #2438 +/- ##
=====================================================
+ Coverage 38.27% 39.07% +0.79%
- Complexity 1809 1877 +68
=====================================================
Files 598 600 +2
Lines 18410 18812 +402
Branches 1232 1275 +43
=====================================================
+ Hits 7047 7351 +304
- Misses 10978 11065 +87
- Partials 385 396 +11 ☔ View full report in Codecov by Sentry. |
22e46c8
to
9e05ec2
Compare
There are some files unrelated to the release changes. |
d286a2c
to
2dccf4f
Compare
@RomuDeuxfois My bad, forgot to do a fetch before creating the branch. It's better now :) |
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.
It looks ok as it is, a few suggestions to reduce the amount of repetition in strings and dicts that are similar
# Modify the release note | ||
logging.info("[platform] Getting the current release note") | ||
release = requests.get( | ||
"https://api.github.com/repos/OpenBAS-Platform/openbas/releases/latest", |
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.
Most URLs in the script would benefit from being built from a set of constants in this script.
A better but more involved chage would be to pass variables from the build environment (e.g. ${{ github.server_url }}/${{ github.repository }}/
) but that's perhaps overkill for this first version.
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.
Totally valid criticism but this code come from splitting the existing python script into several smaller scripts and we intend to move away from them soon so while it would make sense, I feel we should direct our energy toward removing these scripts rather than make them better. wdyt ?
headers={ | ||
"Accept": "application/vnd.github+json", | ||
"Authorization": "Bearer " + github_token, | ||
"X-GitHub-Api-Version": "2022-11-28", |
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.
Headers are always the same: constant ?
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.
Same as #2438 (comment)
adae8cc
to
6efff2f
Compare
Creation of a new release workflow with each repo being owner of their release process