-
Notifications
You must be signed in to change notification settings - Fork 123
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
Packit configuration maintenance #3024
Conversation
0246f4f
to
3b67664
Compare
I have tried 2 approaches to add a separate |
/packit build |
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.
+1 for de-duplicating the actions
items. It's worth mentioning, that the top-level actions will be inherited by all the jobs, though overriding means overriding the whole actions section.
So if I understand it correctly, the yaml anchors are being used to deduplicate the steps when overriding, like:
actions:
<<: *base-actions
get-current-version:
- foo
If that's the case, isn't it redundant for example at https:/teemtee/tmt/blob/3b6766482edf6d48cdc4da0e1f72e95796f8152a/.packit.yaml#L141C5-L141C27, where we're not overriding it?
Indeed, you're right. Similar with For now, let's check that the configuration is correct. At first glance without the |
3b67664
to
16bcbf0
Compare
/packit build |
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.
Nice clean-up, thanks!
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.
Thanks for the deduplication. Looks good, I just noticed we're missing the BusinessUnit
part for one of the internal jobs. Also in c523068 I'm proposing to keep a at least a short comment for each job.
- Use yaml anchors - Remove explicit `enable_net: false` (not needed since June 2022) Signed-off-by: Cristian Le <[email protected]>
16bcbf0
to
c523068
Compare
The copr release job needs to explicitly override `base-actions` otherwise it would inherit them from the `copr_build` definition.
I believe that the release copr build actually needs to explicitly define Without the change the resulting data are: "actions": {
"get-current-version": [
"bash -c \"hatch version | sed -E 's/\\\\.[0-9]+\\\\.dev.*/.dev888/'\""
], With it looks as expected: "actions": {
"get-current-version": [
"hatch version"
], @LecrisUT, @martinhoyer, does it look ok to you? |
/packit test |
Aaah yes, forgot I redefined it in the parent |
/packit build |
Nice idea! |
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.
With the latest changes should be ok.
- Use yaml anchors - Remove explicit `enable_net: false` (not needed since June 2022) - Add missing `BusinessUnit` to the virtual provision job Signed-off-by: Cristian Le <[email protected]> Co-authored-by: Petr Šplíchal <[email protected]>
duplicate copr_build to unblock all other tmt build/runsDoes not work :(copr_build
jobs?Pull Request Checklist
No effective changes yet, other than the duplication of a pr copr_build to see if it unblocks the non-
use_internal_tf
jobsYou can see the full configurations with
packit validate-config
. Comparing them is a nightmare, but you can use jsondiff.com or locally vimdiff to compare these, just get rid of the 2nd job in the "New configuration" in order to diff it more reliablyOriginal configuration
New configuration