Add .pre-commit-config.yaml #175
Conversation
There was a problem hiding this comment.
The git-commit is failing :(
Specifically:
invitations/adapters.py:7:1: F401 'django.utils.encoding.force_str' imported but unused
invitations/adapters.py:28:25: F821 undefined name 'force_text'
(i.e. force_text should be force_str)
Opinion: at this point, we keep the git-config - and this PR - small.
There were changes in practically all pages because of Black, which makes reviewing it super hard. I will for sure let something slip by.
Opinion: this is to be ignored if the PR is kept small, but I would remove migrations files from getting styled by Black as these are auto-generated
This will be fixed when merging #174 either before or after this PR.
I'm thinking that's why we have tests?
Good point! Adding it now! |
Tests are only good for detecting things we considered testing though. I'm not very familiar with all those hooks. It makes me feel uncomfortable about them doing something unpredictable. Btw |
I can vouch for the pre-commit configuration not doing any harm - I've been running with it on other project for quite some time now. The benefits of add-trailing-comma can be read here: https://github.com/asottile/add-trailing-comma#multi-line-method-invocation-style----why |
|
But Black already does all those benefits listed at those docs. Am I missing anything?
…________________________________
From: Víðir Valberg Guðmundsson ***@***.***>
Sent: Wednesday, March 30, 2022 5:12 PM
To: jazzband/django-invitations ***@***.***>
Cc: Fernando ***@***.***>; Review requested ***@***.***>
Subject: Re: [jazzband/django-invitations] Add .pre-commit-config.yaml (PR #175)
I'm not very familiar with all those hooks. It makes me feel uncomfortable about them doing something unpredictable. Btw add-trailing-comma seems unnecessary with black in there.
I can vouch for the pre-commit configuration not doing any harm - I've been running with it on other project for quite some time now.
The benefits of add-trailing-comma can be read here: https://github.com/asottile/add-trailing-comma#multi-line-method-invocation-style----why
—
Reply to this email directly, view it on GitHub<#175 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AE5E6K7PW53W5NT42X7GTDTVCR4O7ANCNFSM5SCHHZLQ>.
You are receiving this because your review was requested.Message ID: ***@***.***>
|
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Black reacts to trailing commas (by putting code with trailing commas on multiple lines) - add-trailing-comma adds them :) |
|
@MrCordeiro @brylie Everything is green now :) |
|
But look at the Black docs: it adds a comma too.
Anyways this is a non blocker. If it's green, it's good. |
Ahh that is just if the line is too long and has to wrapped. add-trailing-comma does it whatever the length of the line. This ties into https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html?highlight=trailing%20comma#the-magic-trailing-comma Anyway, great! I'll merge it! |
No description provided.