Pre commit full

[jpodivin]

All currently defined linters were applied to the repo and exclusion pattern was removed to make sure we maintain this level of coverage.

In several cases, the linters were not behaving in a sensible manner and their settings had to be adjusted.

This PR should be considered with care, because once established the standard of code may be difficult to change.

[nikromen]

This PR should be considered with care, because once established the standard of code may be difficult to change.

I agree... the pre-commit file as it is in the project was only for my usage so I don't have to format the code manually. Extending and enforcing it globally needs approval also from @praiskup and @FrostyX. But since we don't plan to make changes in this repo this week we could discuss it next Tuesday on mtg

[TomasTomecek]

Wow, impressive!

Agreed with @nikromen to discuss this PR on the next meeting.

I will review it in detail on Monday.

[FrostyX]

Thank you for the changes @jpodivin,

This PR should be considered with care, because once established the standard of code may be difficult to change.

Are any other formatting options available or this is the standard? :-)

There are some minor things like {% extends "layout.html" %} {%block content %} or the number of blank lines between headings in markdown that I preferred before this change, so if the linters give us any choices, those would be my preferences.

But I don't really mind. Adhering to any standard is better than chaos.

[jpodivin]

Thank you for the changes @jpodivin,

This PR should be considered with care, because once established the standard of code may be difficult to change.

Are any other formatting options available or this is the standard? :-)

There are some minor things like {% extends "layout.html" %} {%block content %} or the number of blank lines between headings in markdown that I preferred before this change, so if the linters give us any choices, those would be my preferences.

But I don't really mind. Adhering to any standard is better than chaos.

Standard is a loaded term. PEP8 is sometimes considered a standard, but even the PEP8 itself states that it shouldn't be taken slavishly [0]. This is one of the reasons I, personally, don't like opinionated linters.
Checks for syntax are fine, but various prettifiers are, imho, not as useful.

In fact they can become a detriment. As they kick in the moment you hit commit and suddenly you have to go back to check what they did, add their changes to yours, and then hit commit again. It just breaks the flow.

My recommendation would be:

• Checks - yes
• symlinks - yes
• secret detector - yes
• pretty formatters - nope
• flake8 - yes but with some changes, most importantly the line length. 80 is absurdly few chars, I think about 100 fits on most screens.

[0] https://peps.python.org/pep-0008/#a-foolish-consistency-is-the-hobgoblin-of-little-minds

[nikromen]

sum up from what we agreed on the meeting:

• we don't want opinionated automatic formatters (thus deleting darker for formatting python code, pretty-format-json for jsons and prettier for basically everything :D)
• we are OK with the rest of linters/checkers

I think this is close to what you Jirko suggested. Could you please update the PR?

[nikromen]

flake8 - yes but with some changes, most importantly the line length. 80 is absurdly few chars, I think about 100 fits on most screens.

I don't have an opinion on this, both 100 or 88 is ok with me :D

[jpodivin]

sum up from what we agreed on the meeting:

* we don't want opinionated automatic formatters (thus deleting `darker` for formatting python code, `pretty-format-json` for jsons and `prettier` for basically everything :D)

* we are OK with the rest of linters/checkers

I think this is close to what you Jirko suggested. Could you please update the PR?

Final patch takes these into account. Github checks should sync with the new settings automatically starting with this PR.

[FrostyX]

@nikromen you understand this much better than me. Can you please review if this matches what we wanted and merge if yes?

[nikromen]

lgtm