fedora-copr / log-detective-website

Data collection page for Log Detective AI
11 stars 8 forks source link

Pre-commit application #28

Closed jpodivin closed 9 months ago

jpodivin commented 9 months ago

These changes should help us keep certain coding standard while the project grows.

nikromen commented 9 months ago

I agree that we should take advantage of the fact that the project is new and agree on some linters to use for the project to have a uniform style. But this pre-commit file is a set of linters for backend that I use myself, mainly because I don't like to format code manually and don't force its use on everyone - that's why it uses darker instead of black - it formats only changed code in the commit.

I suggest a few things to change in pre-commit config file:

@praiskup @FrostyX WDYT

jpodivin commented 9 months ago

From my experience it's often a difficult task to ensure linters don't become a nuisance. As such, I don't really like linters that format code automatically, rather than checking it, and telling you what to do. But I'm very much open any approach chosen by consensus on this.

What I would warn against, is letting linter list bloat over time. The pre-commit check should be fast, and without many dependencies. It should be something we don't really need to think about. With that in mind, we should agree on a list that's as final as possible.

FrostyX commented 9 months ago

Thank you for the PR @jpodivin, I am sorry I was on a PTO so I couldn't reply in time.

Just to be sure I understand this change correctly - What it does is running linters on GitHub side for every PR? It won't do anything when running git commit on my machine, and it won't do any automatic formatting that @nikromen was suggesting?

Those two things would be a -1 from me. Running linters would be big +1 :-)

FrostyX commented 9 months ago

Also a question maybe for you, maybe my future self. Will the linter complain only about issues introduced in the relevant PR or about all historical issues in the codebase? I am asking because sometimes one has a good enough reason to consciously deviate from the standard.