Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #217 +/- ##
==========================================
+ Coverage 87.62% 92.37% +4.74%
==========================================
Files 33 40 +7
Lines 1543 1822 +279
==========================================
+ Hits 1352 1683 +331
+ Misses 191 139 -52 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| """ | ||
| Generate the table format with collapsible region details | ||
| """ | ||
| output = "| Variant | Platform | Architecture | Flavor | Regions & Image IDs | Download Links |\n" |
There was a problem hiding this comment.
Why not use a module for this? Could something like PrettyTable or pytable be a better solution?
There was a problem hiding this comment.
That would be probably (maybe) easier to read, but I'm not sure if need to bring another dependency because of that.
| @@ -0,0 +1,12 @@ | |||
| from . import DeploymentPlatform | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
Could very well be.
| "usi": "USI (Unified System Image)", | ||
| "tpm2_trustedboot": "TPM2 Trusted Boot", | ||
| } | ||
| PLATFORMS = { |
There was a problem hiding this comment.
That's an interesting take. Maybe we should add some documentation about why we do this? Couldn't we create this not with enums instead?
There was a problem hiding this comment.
Dict is used here simply for grouping platform objects.
| if variant not in grouped_data: | ||
| continue | ||
|
|
||
| for platform in sorted(grouped_data[variant].keys()): |
There was a problem hiding this comment.
I think we could write more effective code here by using the itertools module instead.
There was a problem hiding this comment.
Agree, but suggest to do it as a separate improvement PR.
| continue | ||
|
|
||
| output += ( | ||
| f"<details>\n<summary>Variant - {IMAGE_IDS_VARIANT_NAMES[variant]}</summary>\n\n" |
There was a problem hiding this comment.
With Python 3.14, they stabilized the template string. I feel like I might benefit from using it here.
There was a problem hiding this comment.
Let's keep it in mind, but 3.14 is too fresh to demand it from the library users.
| return "openstackbaremetal" | ||
|
|
||
| def full_name(self): | ||
| return "OpenStack Baremetal" |
There was a problem hiding this comment.
Don't we miss the published_images_by_regions here?
There was a problem hiding this comment.
For some platforms superclass' version is used.
|
|
||
|
|
||
| def test_script_parse_args_wrong_command(monkeypatch, capfd): | ||
| monkeypatch.setattr(sys, "argv", ["gh", "rejoice"]) |
There was a problem hiding this comment.
Do we call this tool gh? That is confusing, and we have to change it; otherwise, it will collide with the gh cli tool.
There was a problem hiding this comment.
No, this command name is used just for testing. The actual script name for releases creation has to be added to pyproject.toml's tool.poetry.scripts section.
2 participants
What this PR does / why we need it:
Splitting #179 into smaller parts
Which issue(s) this PR fixes:
gardenlinux/gardenlinux#3054