secure-systems-lab / securesystemslib

Cryptographic and general-purpose routines for Secure Systems Lab projects at NYU
MIT License
46 stars 48 forks source link

Auto-update pre-commit plugins #734

Open lukpueh opened 5 months ago

lukpueh commented 5 months ago

Plugins (most notably isort and black) in .pre-commit-config.yaml are not updated by Dependabot and thus prone to become out of sync with the same dev tools used by tox (also see discussion with @jku in #582).

We should find a way to keep them in-sync.

jku commented 5 months ago

without understanding pre-commit much, how about this:

I think this would explicitly not use pre-commit itself though.

lukpueh commented 5 months ago

The difference would be that tox -e lint doesn't reformat your code.

jku commented 5 months ago

A tox -e reformat command would be nice to have anyway maybe

lukpueh commented 5 months ago

I'm always pro lighter toolchain.

L77H commented 1 month ago

With Ruff as our linter now we can add tox -e reformat with ruff format {[testenv:lint]lint_dirs} in tox.ini. We could also add this immediately on tox -e lint by removing the --diff flag on ruff format, this would depend if the suggested reformatting changes should be reviewed first before being performed.

lukpueh commented 1 month ago

In my experience, reformatting and format checking + linting are usually separated. But I suppose we can combine them, now that both of them are so fast. The important thing is that tox -e lint fails CI, if the format is incorrect. What do others think?

jku commented 1 month ago

I think the autofix/autoformat feature is not a must-have but I'm not complaining if it's there. I wouldn't add it to lint itself though -- it feels like there should be a way to "just test", not fix.

In python-tuf I added tox -e fix which ends up running something like

ruff check --fix <DIRS>
ruff format <DIRS>
lukpueh commented 1 month ago

I wouldn't add it to lint itself though -- it feels like there should be a way to "just test", not fix.

Agreed. Not so much because I want to be able just test, but more because I don't want change anyone's sources when they don't expect it.