training-operator icon indicating copy to clipboard operation
training-operator copied to clipboard
verified publisher icon kubeflow

Implement pre-commit hooks

Open droctothorpe opened this issue 1 year ago • 8 comments

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 # https://github.com/kubeflow/training-operator/issues/2178

Checklist:

  • [x] Docs included if any changes are user facing

droctothorpe avatar Jul 25 '24 19:07 droctothorpe

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

review-notebook-app[bot] avatar Jul 25 '24 19:07 review-notebook-app[bot]

@andreyvelich 🎉

droctothorpe avatar Jul 25 '24 19:07 droctothorpe

Pull Request Test Coverage Report for Build 10182830151

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.1%) to 34.944%
Totals Coverage Status
Change from base Build 10131041132: 0.1%
Covered Lines: 3949
Relevant Lines: 11301
💛 - Coveralls

coveralls avatar Jul 25 '24 19:07 coveralls

A few autogenerated files slipped through the cracks. Addressing now. Also going to move the black execution to the pre-commit hooks.

droctothorpe avatar Jul 25 '24 20:07 droctothorpe

Fixed. All checks passing.

droctothorpe avatar Jul 25 '24 20:07 droctothorpe

The integration test failures are unrelated to this PR afaict. They both stop logging after Collecting urllib3>=1.15.1 (from kubeflow-training==1.8.0).

droctothorpe avatar Jul 25 '24 22:07 droctothorpe

@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.

droctothorpe avatar Jul 30 '24 15:07 droctothorpe

/rerun-all

andreyvelich avatar Aug 05 '24 16:08 andreyvelich

[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 /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

google-oss-prow[bot] avatar Aug 05 '24 16:08 google-oss-prow[bot]

Thanks for the merge, @andreyvelich! Happy to add more hooks and also bring the katib hooks into alignment.

droctothorpe avatar Aug 05 '24 17:08 droctothorpe