Proposal: Add ruff (linter and formatter) #40
Conversation
SmittieC
requested review from
kaapstorm,
sravfeyn,
mkangia,
Charl1996 and
zandre-eng
SmittieC
changed the title
There was a problem hiding this comment.
I love this idea. I am a fan of ruff. I use it in my own projects. It's the best linter.
But I will push back hard on a couple of those formatting choices: Either use single quotes, or leave the quotes alone. Also, these bracket decisions do not improve readability.
hq_superset/__init__.py
Outdated
| template_path = os.sep.join( | ||
| (os.path.dirname(os.path.abspath(__file__)), "templates") | ||
| ) |
hq_superset/api.py
Outdated
| @@ -36,8 +36,8 @@ def save_token(token, request): | |||
| tok = Token( | |||
| client_id=client.client_id, | |||
| expires_at=expires_at, | |||
| access_token=token['access_token'], | |||
| token_type=token['token_type'], | |||
| access_token=token["access_token"], | |||
There was a problem hiding this comment.
You not a fan of double quotes?
There was a problem hiding this comment.
You might find a840441 to your liking?
pyproject.toml
Outdated
| @@ -0,0 +1,5 @@ | |||
| [tool.ruff] | |||
| line-length = 120 | |||
There was a problem hiding this comment.
I can't remember, do we as dimagi use 120 chars as the line length?
There was a problem hiding this comment.
For flake8, it's currently set to 115 lines for CommCare HQ (code reference)
There was a problem hiding this comment.
We use 115 (to make code review easier (if your browser is maximized on a 1280-wide screen)).
(Personally, I prefer PEP-8 line lengths (79 for code, 72 for docstrings) because (1) it makes code readable when you split your windows vertically -- code on the left, tests on the right -- and (2) long lines of text are ugly and hard to read, which is why newspapers have columns.)
pyproject.toml
Outdated
| @@ -0,0 +1,5 @@ | |||
| [tool.ruff] | |||
| line-length = 115 | |||
There was a problem hiding this comment.
Curious what you think of this:
| line-length = 115 | |
| line-length = 79 # Preferred line length to wrap lines | |
| [tool.ruff.lint.pycodestyle] | |
| max-line-length = 115 # Line length to allow for line-too-long violations |
I noticed that Ruff will not reformat strings if they exceed line-length.
There was a problem hiding this comment.
79 is probably fine.
I noticed that Ruff will not reformat strings if they exceed line-length.
Weird
| line-length = 115 | ||
|
|
||
| [tool.ruff.format] | ||
| quote-style = "single" |
There was a problem hiding this comment.
I noticed that Ruff has a "preserve" option to leave quotes as-is, but that it prefers to follow PEP-8 which says, "Pick a rule and stick to it." So, okay, fair enough.
There was a problem hiding this comment.
I find it ironic that "single" is in double quotes.
There was a problem hiding this comment.
Yeah that's my reasoning as well. I don't mind single or double quotes. I presume you prefer single quotes?
There was a problem hiding this comment.
Oh I see you said it just below: "I prefer single"
There was a problem hiding this comment.
On principle I think this is a good idea. I think this is also a good codebase to use this in. And I'm coming around to liking the idea of having only one choice of quotation marks. I prefer single, but I could also make an argument in favor of double. ... I do like 79-character lines though, because it makes my IDE layout happy.
Cool. I don't have any objections to 79 character lines. I'll make an update |
|
Do we still need isort on top of this? I like that it fixes files automatically Does ruffle also take care of line spacing between methods? I have always used 2 spaces and now assume that is the standard. |
|
PR is still in draft though. |
|
FYI: I snuck d1ee5ae into this PR. |
Looks like we can get sorting with 8aec7d9. As for the spaces between methods, I'm not 100% sure. Will have to look into this a bit more, but I'm sure one can achieve this. |
Great, thanks |
|
@SmittieC What is the status of this PR? |
I forgot about this. If people are happy I think we can merge |
Things will get ruff if we don't add something like this sooner or later. This is a linter and formatter. This is apparently way faster than tools like
blackandflake8.I also added a Makefile to reduce the amount of commands one has to run each time working with this.
It's base branch is my latest branch based off of
nh/webhook. This is to avoid as much merge conflicts as possible if/when this is merged