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

Enable ruff's flake8-commas rule #3044

Merged
merged 4 commits into from
Aug 30, 2024

Conversation

CoolCat467
Copy link
Member

@CoolCat467 CoolCat467 commented Aug 1, 2024

In this pull request, we enable ruff's flake8-commas rule and handle all the reformatting black does because of this.

No functional changes Had to update gen_exports to run black a 2nd time, if/when this is merged it will probably need to be added to the purely formatting commits thing jakkdl was talking about a while ago.

Copy link

codecov bot commented Aug 1, 2024

Codecov Report

Attention: Patch coverage is 25.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 99.60%. Comparing base (4ae737a) to head (30babaf).
Report is 236 commits behind head on main.

Files with missing lines Patch % Lines
src/trio/_tools/gen_exports.py 25.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3044      +/-   ##
==========================================
- Coverage   99.61%   99.60%   -0.02%     
==========================================
  Files         121      121              
  Lines       17880    17884       +4     
  Branches     3214     3215       +1     
==========================================
+ Hits        17812    17813       +1     
- Misses         48       50       +2     
- Partials       20       21       +1     
Files with missing lines Coverage Δ
src/trio/_abc.py 100.00% <ø> (ø)
src/trio/_channel.py 100.00% <ø> (ø)
src/trio/_core/_asyncgens.py 100.00% <ø> (ø)
src/trio/_core/_concat_tb.py 100.00% <ø> (ø)
src/trio/_core/_entry_queue.py 100.00% <ø> (ø)
src/trio/_core/_io_epoll.py 100.00% <ø> (ø)
src/trio/_core/_io_kqueue.py 87.20% <ø> (ø)
src/trio/_core/_io_windows.py 98.80% <ø> (ø)
src/trio/_core/_parking_lot.py 100.00% <ø> (ø)
src/trio/_core/_run.py 99.38% <ø> (ø)
... and 71 more

@jakkdl
Copy link
Member

jakkdl commented Aug 5, 2024

In this pull request, we enable ruff's flake8-commas rule and handle all the reformatting black does because of this.

No functional changes Had to update gen_exports to run black a 2nd time, if/when this is merged it will probably need to be added to the purely formatting commits thing jakkdl was talking about a while ago.

#2837

@CoolCat467
Copy link
Member Author

pre-commit.ci autofix

@CoolCat467 CoolCat467 added the skip newsfragment Newsfragment is not required label Aug 21, 2024
Copy link
Contributor

@A5rocks A5rocks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spot-checks fine. I'm not sure how useful this will be, though forcing noqa and type: ignores onto lines with only that argument is actually useful.

Though, can you put this in a blame-ignore file?

@jakkdl
Copy link
Member

jakkdl commented Aug 27, 2024

Spot-checks fine. I'm not sure how useful this will be, though forcing noqa and type: ignores onto lines with only that argument is actually useful.

Though, can you put this in a blame-ignore file?

it's also generally useful for minimizing diffs when only changing a single argument.

And yes, this should be in blame-ignore. If you wanna be strictly correct you can reshuffle the commits so there's one that is pure refactoring, and one that contains the functional changes to gen_exports. This can for example be done with something like (untested, and might depend on the state of your local repo)

git reset origin/main
git stash push --patch src/trio/_tools/gen_exports.py
[interactively only stash the functional changes]
git commit -am [...]
git stash pop
git commit -am [...]

after some brief testing it appears it's possible to add the hash to .git-blame-ignore-revs and not have it change if the PR is merged with a merge commit, in which case you can also do that in the second commit (or in a third).

If you mess up, there's git reflog, or pulling from the remote branch. If you don't manage to work it out lmk and I can do it :)

@CoolCat467 CoolCat467 force-pushed the enable-flake8-commas branch from 351f6ca to 810534d Compare August 29, 2024 15:44
@CoolCat467 CoolCat467 merged commit 0090581 into python-trio:main Aug 30, 2024
33 of 35 checks passed
@jakkdl
Copy link
Member

jakkdl commented Aug 31, 2024

2024-08-31T13:03:13,100427169+02:00
https://github.com/python-trio/trio/blame/main/docs/source/conf.py
whoop whoop!

@CoolCat467 CoolCat467 deleted the enable-flake8-commas branch September 1, 2024 01:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip newsfragment Newsfragment is not required
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants