*.py files migrated from gardenlinux workflows#179
*.py files migrated from gardenlinux workflows#179vivus-ignis wants to merge 95 commits intomainfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #179 +/- ##
==========================================
- Coverage 92.95% 92.43% -0.53%
==========================================
Files 28 30 +2
Lines 1306 1797 +491
==========================================
+ Hits 1214 1661 +447
- Misses 92 136 +44 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| description: "Generated GitHub workflow flavors matrix" | ||
| version: | ||
| description: GardenLinux Python library version | ||
| default: "0.10.0" |
There was a problem hiding this comment.
Please remove the version here. Don't see an benefit with parametrizing it here.
pyproject.toml
Outdated
| gl-flavors-parse = "gardenlinux.flavors.__main__:main" | ||
| gl-oci = "gardenlinux.oci.__main__:main" | ||
| gl-s3 = "gardenlinux.s3.__main__:main" | ||
| gl-gh = "gardenlinux.github.__main__:main" |
There was a problem hiding this comment.
The last review request is still valid.
| @@ -0,0 +1,3 @@ | |||
| from .__main__ import create_github_release_notes | |||
|
|
|||
| __all__ = ["create_github_release_notes"] | |||
There was a problem hiding this comment.
IMHO there's no reason to export this function.
There was a problem hiding this comment.
In my opinion calling gardenlinux.github.create_github_release_notes looks much better than gardenlinux.github.main.create_github_release_notes.
There was a problem hiding this comment.
While I do agree in general you won't call create_github_release_notes() from any other source than the main entrypoint.
src/gardenlinux/github/__main__.py
Outdated
|
|
||
| s3_artifacts._bucket.download_file( | ||
| release_object.key, artifacts_dir.joinpath(f"{cname}.s3_metadata.yaml") | ||
| ) |
There was a problem hiding this comment.
Still would like to see s3_artifacts.bucket here and everywhere else where used. Reasoning given last time.
src/gardenlinux/github/__main__.py
Outdated
| def download_all_metadata_files(version, commitish): | ||
| repo = Repo(".") | ||
| commit = repo.commit(commitish) | ||
| flavors_data = commit.tree["flavors.yaml"].data_stream.read().decode("utf-8") |
There was a problem hiding this comment.
I'm still not sure if flavors.yaml should be "checked" out here.
As you are touching a local Git repository here I'm not sure if it is useful to reset the checked out commit each time this function is called. You may just access "whatever"
flavors.yamlis located here instead. Please be aware thatFlavorsParserwill use thefeaturesdirectory that is located here anyway without ensuring that the commitish matches.
src/gardenlinux/github/__main__.py
Outdated
| commitish_short = commitish[:8] | ||
|
|
||
| for flavor in flavors: | ||
| cname = CName(flavor[1], flavor[0], "{0}-{1}".format(version, commitish_short)) |
There was a problem hiding this comment.
Same request as last time:
You may use the
commit_idparameter recently added here.
src/gardenlinux/github/__main__.py
Outdated
| def release_notes_compare_package_versions_section(gardenlinux_version, package_list): | ||
| output = "" | ||
| version_components = gardenlinux_version.split(".") | ||
| # Assumes we always have version numbers like 1443.2 |
There was a problem hiding this comment.
Please be aware we'll introduce semantic versioning soon. This logic should use a
semverPython package if suitable and consider the patch version to be zero if only one dot is found in the string.
|
@vivus-ignis please cherry-pick the changes from gardenlinux/gardenlinux#3485 |
Done. |
|
Hate to be the party pooper here, but this PR is far too big. There are many different type changes that this PR contains that can be split up easier. I get that this is a refactor, but we can split it into more mindful pieces. Please split this PR into several small PRs. For instance, test data as a standalone PR. Then there is We change the exports from the Why do we add a binary file to this PR? Can we skip this, or can we generate it? |
|
This is not relevant anymore as the changes were integrated with the code base by a series of separate smaller PRs. |
As agreed with @Akendo, splitting this PR into smaller PRs.
What this PR does / why we need it: This consolidates python CI code from the main gardenlinux repository into the existing python module
Which issue(s) this PR fixes:
Fixes
Special notes for your reviewer:
Release creation code was tested on a gardenlinux repo fork: https://github.com/vivus-ignis/gardenlinux-dev/releases
As the bulk of the code was extracted from the gardenlinux/gardenlinux repository, this PR does not show what was changed there. Therefore attaching a diff here:
Diff
Release note: