Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auto-update pre-commit plugins #734

Open
lukpueh opened this issue Feb 14, 2024 · 8 comments
Open

Auto-update pre-commit plugins #734

lukpueh opened this issue Feb 14, 2024 · 8 comments
Labels
discussion Issues that require discussion

Comments

@lukpueh
Copy link
Member

lukpueh commented Feb 14, 2024

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
Copy link
Collaborator

jku commented Feb 14, 2024

without understanding pre-commit much, how about this:

  • make tox -e lint fast (aka use ruff)
  • start running tox -e lint in a commit hook

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

@lukpueh
Copy link
Member Author

lukpueh commented Feb 14, 2024

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

@jku
Copy link
Collaborator

jku commented Feb 14, 2024

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

@lukpueh
Copy link
Member Author

lukpueh commented Feb 14, 2024

I'm always pro lighter toolchain.

@lukpueh lukpueh added the discussion Issues that require discussion label Mar 14, 2024
@L77H
Copy link
Contributor

L77H commented Jul 1, 2024

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
Copy link
Member Author

lukpueh commented Jul 2, 2024

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
Copy link
Collaborator

jku commented Jul 2, 2024

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
Copy link
Member Author

lukpueh commented Jul 2, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Issues that require discussion
Projects
None yet
Development

No branches or pull requests

3 participants