search

scipp / scippnexus

h5py-like utility for NeXus files with seamless scipp integration
https://scipp.github.io/scippnexus/
BSD 3-Clause "New" or "Revised" License
3 stars 3 forks source link

Pre commit in ci #93

Closed jl-wynen closed 1 year ago

jl-wynen commented 1 year ago

I cleaned up the commits to avoid clutter. But this setup seems to work. It runs all pre-commit checks. If static analysis fails, it stops the workflow. If files get reformatted, it commits the result. And as before, CI keeps running with the old files and never with the autocommited ones.

jl-wynen commented 1 year ago

Note for future reference, that it would be nicer to run it like this in a shell pre-commit run -a || pre-commit run -a, because it only runs the 2nd command if the 1st fails. But tox does not seem to support this.

SimonHeybrock commented 1 year ago

Why not something like

commands =
  sh -c 'pre-commit run -a || pre-commit run -a'
jl-wynen commented 1 year ago

Works. I tried something like this before and it gave a syntax error in tox. I don't remember what I did differently.

I also changed the order of checks in .pre-commit-config.yaml such that formatters run before analysers. This means, e.g., that flake8 does not report on formatting issues that yapf can fix.

SimonHeybrock commented 1 year ago

I also changed the order of checks in .pre-commit-config.yaml such that formatters run before analysers. This means, e.g., that flake8 does not report on formatting issues that yapf can fix.

Can you make the same change in scipp please? Thanks!

jl-wynen commented 1 year ago

Can you make the same change in scipp please? Thanks!

Yes. I am going to roll this out everywhere.