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

Use pre-commit for linting and code consistency? #43

Open
snomos opened this issue Jan 30, 2024 · 2 comments
Open

Use pre-commit for linting and code consistency? #43

snomos opened this issue Jan 30, 2024 · 2 comments
Labels
enhancement New feature or request

Comments

@snomos
Copy link
Member

snomos commented Jan 30, 2024

https://pre-commit.com

Works locally as well as part of CI.

WDYT @albbas @flammie @Phaqui ?

@snomos snomos added the enhancement New feature or request label Jan 30, 2024
@Phaqui
Copy link
Contributor

Phaqui commented Jan 30, 2024

Sure. Just a few observations: To avoid creating more noise than it clears, everyone who commits to the repo should be on board, and use pre-commit, with the same hooks (defined in .pre-commit-config.yaml). The rules should also be on the softer side.

Scenario:
Pretext: Person A does not use pre-commit. Person B does.

Person A commits non-formatted code. Person B pulls, and makes some changes. When B commits, A's unformatted changes will be included in B's commit, because B will format all code (unless B goes to some lengths to explicitly prevent it). This can look noisy.


It can cause more hassle than it alleviates if the pre-commit rules are too strict. Sometimes, the linter (or even code formatter) may not agree with some code - even though you specifically need it that way. It can be a good idea to at least start it off on the less restrictive side. Maybe start with only code formatting, and no linting altogether?

@flammie
Copy link
Contributor

flammie commented Jan 30, 2024

Yeah my past experience from trying to get project-wide pre-commit hooks is that they usually end up like described above, if it blocks commit (push) it can leave people who didn't break the code unable to work or annoyed. If it can be efficiently enforced which it looks like might be more possible nowadays it's a very good idea. I've used for myself in the past effectively.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants