Add -c option to validate-config #2556
Conversation
|
Build failed. ❌ pre-commit FAILURE in 2m 06s |
packit/cli/validate_config.py
Outdated
| except FileNotFoundError as e: | ||
| logger.error(f"Error: {e}") |
There was a problem hiding this comment.
We already check for this at line 68 so this should not be needed.
packit/cli/validate_config.py
Outdated
| except FileNotFoundError as e: | ||
| logger.error(f"Error: {e}") | ||
| except yaml.YAMLError as e: | ||
| logger.error(f"YAML parsing error: {e}") | ||
| except Exception as e: | ||
| logger.error(f"Unexpected error: {e}") |
There was a problem hiding this comment.
It would be nice if we could resolve these error states and fail soo. (It helps code readability if we don't go deeper (~right) into code structure.)
8d0e2c1 to
ca3c875
Compare
|
@lachmanfrantisek okay i added test cases and resolve to fail fast also i tried to implement the TODO debug |
|
Build failed. ❌ pre-commit FAILURE in 2m 03s |
|
Build failed. ❌ pre-commit FAILURE in 2m 13s |
|
Build succeeded. ✔️ pre-commit SUCCESS in 2m 13s |
bf51b3b to
7ad7486
Compare
|
Build succeeded. ✔️ pre-commit SUCCESS in 2m 17s |
|
@mfocko can you please review those changes |
|
And btw. can you please also (Also, an interactive rebase might be nice to clean the history so each commit is a logical part of the contribution without any unnecessary/work-in-progress/merging/fixup... commits.) Thanks! |
7ad7486 to
2ad6ee5
Compare
|
@lachmanfrantisek i moved the logic to validate_config fucniton in PackitAPI class and make tests with flexmock on this function |
|
Build succeeded. ✔️ pre-commit SUCCESS in 2m 04s |
|
Build succeeded. ✔️ pre-commit SUCCESS in 2m 06s |
There was a problem hiding this comment.
Sounds good!
Just one code suggestion.
Otherwise, the last step is to clean up the git history a bit:
- Can you please rebase your code instead of merging it so we have a clean history? (The commit(s) will start on the current
mainbranch HEAD.) - If you setup pre-commit (documented here, you can avoid attaching the fix commit by the CI.
- Also, we don't need the prettier cache file.
Thanks!
|
Build succeeded. ✔️ pre-commit SUCCESS in 2m 07s |
051113a to
7259e0d
Compare
|
Build succeeded. ✔️ pre-commit SUCCESS in 2m 02s |
|
@lachmanfrantisek Done. |
There was a problem hiding this comment.
Just two small things to do:
- One rebase issue (you are trying to revert part of rename validate config to config validate for config grouping #2559)
- It would be nice to use pre-commit locally to incorporate pre-commit fixes in your commit.
packit/cli/validate_config.py
Outdated
| @click.command("validate", context_settings=get_context_settings()) | ||
| @click.command("validate-config", context_settings=get_context_settings()) |
There was a problem hiding this comment.
Looks like some leftovers from the rebase because of #2559
There was a problem hiding this comment.
should i change it manually to be "validate" ?
or there's a better way
e6803ba to
1327a4f
Compare
|
Build failed. ✔️ pre-commit SUCCESS in 2m 07s |
1327a4f to
207b3d0
Compare
|
Build failed. ✔️ pre-commit SUCCESS in 2m 06s |
|
@lachmanfrantisek there’s a CI issue? or i made a problem. |
|
Let's rerun it but it might be caused by a fresh |
|
recheck |
|
Build succeeded. ✔️ pre-commit SUCCESS in 2m 07s |
|
@lachmanfrantisek all good now? or anything else |
1327a4f to
e2e83f7
Compare
|
@lachmanfrantisek Done. |
|
Build succeeded. ✔️ pre-commit SUCCESS in 2m 05s |
|
Sorry for missing the message. I'll take care of the rebasing this time..;) |
e2e83f7 to
822b269
Compare
|
Build failed. ✔️ pre-commit SUCCESS in 2m 12s |
|
@lachmanfrantisek okay no problem but why CI take so long ..;) |
|
taking over as it was supposed to merged |
fix pre-commit Add -c / --config-path option to validate_config and corresponding tests
822b269 to
da483a8
Compare
|
Build succeeded. ✔️ pre-commit SUCCESS in 2m 07s |
|
Build succeeded (gate pipeline). ✔️ pre-commit SUCCESS in 2m 03s |
7368453
into
packit:main
|
Thanks Matěj for taking care of this. And thanks @MooSayed1 for the work on this. |
Fixes #2451
Description
Fixes #2451
i added a
-c / --configoption to thevalidate-configcommand.How to Test (So Far)
RELEASE NOTES BEGIN
You can now validate a Packit config passed to the Packit CLI via a path, e.g.,
packit config validate -c /tmp/my-custom-packit-config.yml.RELEASE NOTES END