move `ruff` and `bandit` checks to `pre-commit`

spacetelescope / jwst

Python library for science observations from the James Webb Space Telescope

https://jwst-pipeline.readthedocs.io/en/latest/

Other

561 stars 167 forks source link

move `ruff` and `bandit` checks to `pre-commit` #8646

Closed zacharyburnett closed 2 months ago

zacharyburnett commented 2 months ago

uses pre-commit for linting, formatting, and security checking. By default, pre-commit is run only on changes lines. Putting these checks in .pre-commit-config.yaml ensures that we can pin the ruff version to make sure that new rules don't break style checks randomly

Checklist for PR authors (skip items if you don't have permissions or they are not applicable)

[x] ~added entry in CHANGES.rst within the relevant release section~
[x] ~updated or added relevant tests~
[ ] updated relevant documentation
[ ] added relevant milestone
[x] added relevant label(s)
[x] ~ran regression tests, post a link to the Jenkins job below. How to run regression tests on a PR~
[ ] All comments are resolved
[x] ~Make sure the JIRA ticket is resolved properly~

braingram commented 2 months ago

I always get this mixed up but does adding this file also mean the pre-commit bot will auto-fix PRs?

braingram commented 2 months ago

I'm also failing to find where pre-commit runs in the CI.

Perhaps we need a new workflow to run pre-commit (perhaps like the one in asdf).

We could also enable the bot (but I don't trust it as you already know).

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 59.60%. Comparing base (ccf871a) to head (f89b828). Report is 7 commits behind head on master.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #8646 +/- ## ========================================== + Coverage 59.56% 59.60% +0.03% ========================================== Files 391 391 Lines 39285 39286 +1 ========================================== + Hits 23402 23418 +16 + Misses 15883 15868 -15 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

braingram commented 2 months ago

:-O I think we should dial back those pre-commit hooks or exclude more files. The log for the pre-commit job is over 50k lines long...

zacharyburnett commented 2 months ago

:-O I think we should dial back those pre-commit hooks or exclude more files. The log for the pre-commit job is over 50k lines long...

yeah it looks like it was ruff format, trim-trailing-whitespace, and end-of-file-fixer. I thought they would only run on changed files, though...