Implement pre-commit hooks #2184
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
droctothorpe
force-pushed
the
pc
branch
2 times, most recently
from
7688437 to
37391ed
Compare
Pull Request Test Coverage Report for Build 10182830151Details
💛 - Coveralls |
|
A few autogenerated files slipped through the cracks. Addressing now. Also going to move the black execution to the pre-commit hooks. |
droctothorpe
force-pushed
the
pc
branch
2 times, most recently
from
3680975 to
b13cb76
Compare
|
Fixed. All checks passing. |
|
The integration test failures are unrelated to this PR afaict. They both stop logging after |
Signed-off-by: droctothorpe <[email protected]>
|
@andreyvelich any chance we could expedite this? The changes are all superficial but the surface area is large so it's going to require constant rebasing. |
There was a problem hiding this comment.
Thank you for doing this for Training Operator @droctothorpe!
/assign @kubeflow/wg-training-leads
.pre-commit-config.yaml
Outdated
| rev: 24.2.0 | ||
| hooks: | ||
| - id: black | ||
| files: sdk/.* |
There was a problem hiding this comment.
Should we also format files in examples directory ?
@kubeflow/wg-training-leads do we have any other location where we store Python files ?
There was a problem hiding this comment.
Added the examples dir to this hook in a standalone commit so that you can review the scope of the changes. I'm in favor of it FWIW. Consistency across the whole codebase ftw.
Signed-off-by: droctothorpe <[email protected]>
Signed-off-by: droctothorpe <[email protected]>
Signed-off-by: droctothorpe <[email protected]>
|
/rerun-all |
There was a problem hiding this comment.
I think, we should be ready to merge this.
Thanks for this work @droctothorpe!
/lgtm
/approve
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: andreyvelich The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Thanks for the merge, @andreyvelich! Happy to add more hooks and also bring the katib hooks into alignment. |
What this PR does / why we need it:
This PR implements pre-commit hooks and ensures that they're executed, as validating admission control, in CI. Once this is merged, I plan to submit follow up PRs that progressively implement additional hooks.
Which issue(s) this PR fixes
Fixes # #2178
Checklist: