lint (black/pyflakes) everything that looks like python code using pre-commit #1461

graingert · 2020-10-22T09:50:15Z

Contributor Checklist:

The associated ticket in Trac is here: https://twistedmatrix.com/trac/ticket/10032
I ran tox -e black-reformat to format my patch to meet the Twisted Coding Standard
I ran additional checks listed at Getting Your Patch Accepted
I have created a newsfragment in src/twisted/newsfragments/ (see: News files)
I have updated the automated tests.
I have submitted the associated Trac ticket for review by adding the word review to the keywords field in Trac, and putting a link to this PR in the comment; it shows up in https://twisted.reviews/ now.

`pipx run modernize -wn docs/historic/2003/pycon/`

…a.tac

graingert · 2020-10-22T09:56:43Z

I was somewhat conflicted on fixing the historic/2003 python docs - but the changes made to make them syntactically valid under python3 are so minimal that I think it's worth doing.

I'd rather delete them than shade them from the light of black

.pre-commit-config.yaml

adiroiban

Wow! Great stuff :)

Happy to see this and I think that this PR is on the right path

A have a few major comments before approving this.

See inline comments.

Thanks!

adiroiban · 2020-10-22T14:32:53Z

.github/workflows/lint.yaml

@@ -10,7 +10,7 @@ jobs:
  lint:
    runs-on: ubuntu-20.04
    env:
-      TOXENV: "lint,black,twine"
+      TOXENV: "lint,twine"


do we still need twine here?

Yep. twine runs other checks on the built wheel and doesn't really fit with the pre-commit model

But I guess this will be removed in the followup PR with the new wheel+sdist build workflow... or am O lost ? ... again :)

I'm keeping twine still in the release PR too

.github/PULL_REQUEST_TEMPLATE

adiroiban · 2020-10-22T15:03:10Z

README.rst

-Or, for speed, specify exactly what to format::
-
-  $ tox -e black-reformat -- path/to/file.py
+  $ pipx run pre-commit run


can we add a tox pre-commit env as the "standard" tool... pipx is great, but I think that is best to keep all in tox.

People who will want to run pipx or just custom virtualenv could then use the rules from tox to see how to run-precommit

tox -e lint -- run does that already

Them maybe suggest

Suggested change

$ pipx run pre-commit run

$ tox -e lint -- run

I would prefer tox -e lint to only run on changes since trunk. and have tox -e lint --all-files to be used by CI.

running via tox instead of pipx adds quite a bit of slowness - pipx here is more of a demo on how it could be installed.

adiroiban · 2020-10-22T15:05:00Z

.pre-commit-config.yaml

+  - repo: https://gitlab.com/pycqa/flake8
+    rev: 3.8.3
+    hooks:
+      - id: flake8


flake8 runs only pyflakes by default ?

yes, flake8 runs pycodestyle and pyflakes, Ive ignored all the pycodestyle violations that currently stand, and double checked the pyflakes violations are not ignored

adiroiban · 2020-10-22T15:07:16Z

I'd rather delete them than shade them from the light of black

I think we can keep them...since they are historic. ... but is ok to update then with black.
If we remove, them I think that should be done in a separate PR.

Thanks!

adiroiban

Thanks. Only minor comments.

This is ready for merge.

setup.cfg

adiroiban · 2020-10-22T16:30:24Z

README.rst

-Or, for speed, specify exactly what to format::
-
-  $ tox -e black-reformat -- path/to/file.py
+  $ pipx run pre-commit run


Them maybe suggest

Suggested change

$ pipx run pre-commit run

$ tox -e lint -- run

I would prefer tox -e lint to only run on changes since trunk. and have tox -e lint --all-files to be used by CI.

admin/fix-for-src-mv.py

Co-authored-by: Adi Roiban <adiroiban@gmail.com>

rodrigc · 2021-01-03T22:26:05Z

@graingert You removed the tox -e black target, but you didn't update the places on the wiki where this target is mentioned: https://twistedmatrix.com/trac/wiki/TwistedDevelopment

Do you plan to update the wiki?

@adiroiban / @wsanchez When reviewing this type of stuff, please double check that any documentation changes on the wiki are updated as well.

adiroiban · 2021-01-03T22:44:57Z

Thanks for the followup. Well, we do our best with the review

I think that anyone can update the Wiki without the need for a review... so feel free to update the wiki page.

I would like to stop using the wiki and instead use the repo's docs for documentation.

In this way, everything can be done in a single PR.

We now have up to date docs based on the trunk branch at

https://docs.twistedmatrix.com/en/latest/core/development/index.html

graingert · 2021-01-03T22:50:23Z

I would like to stop using the wiki and instead use the repo's docs for documentation.

I tend to agree, in this case I think we should delete the outdated wiki version of these docs and link to the updated version on https://docs.twistedmatrix.com/

rodrigc · 2021-01-03T23:57:52Z

@graingert for now, can you update the wiki as I have asked? While not perfect, the wiki has been around for a long time,
and for better or worse, many people in the project, and outside the project still refer to it.

The Twisted Development process is a very important part of the Twisted project. If this is deleted from the wiki
in favor of documentation inside the tree, then this needs to be done via a ticket and PR (just like anything else in Twisted),
so that there is an audit trail that this has been decided and agreed upon.

graingert added 11 commits October 22, 2020 09:54

modernize 2003 pycon talk

3d177ca

`pipx run modernize -wn docs/historic/2003/pycon/`

fix syntax error in docs/core/howto/tutorial/listings/finger/finger19…

e24299c

…a.tac

black everything

1e42d3e

add pre-commit config

de08b09

Strip unnecessary # noqas

c1ec888

ignore all current flake8 violations

59573f1

strip more noqa

545ed9b

split doc and src/twisted flake8 violations

927d520

fix remaining flake8 violation

55631fc

document and run pre-commit with tox

1120d9d

add newsfragment

87c7360

graingert force-pushed the black-everything branch from e9ea0cc to 87c7360 Compare October 22, 2020 09:53

graingert changed the title ~~Black everything~~ lint everything that looks like python code Oct 22, 2020

graingert changed the title ~~lint everything that looks like python code~~ lint (black/pyflakes) everything that looks like python code using pre-commit Oct 22, 2020

graingert added 2 commits October 22, 2020 11:36

reduce max-line-length by at least 50%

0e19a83

reduce max-line-length by at least 50%

2096409

graingert requested a review from adiroiban October 22, 2020 11:03

wsanchez reviewed Oct 22, 2020

View reviewed changes

.pre-commit-config.yaml Outdated Show resolved Hide resolved

Update .pre-commit-config.yaml

3d5bb78

graingert requested a review from wsanchez October 22, 2020 12:14

graingert closed this Oct 22, 2020

graingert reopened this Oct 22, 2020

adiroiban requested changes Oct 22, 2020

View reviewed changes

ignore pre-commit based 'black everything' commit

3f6c7a6

adiroiban approved these changes Oct 22, 2020

View reviewed changes

Update admin/fix-for-src-mv.py

7337303

Co-authored-by: Adi Roiban <adiroiban@gmail.com>

wsanchez approved these changes Oct 22, 2020

View reviewed changes

graingert merged commit d4cf03c into twisted:trunk Oct 22, 2020

graingert deleted the black-everything branch October 22, 2020 17:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lint (black/pyflakes) everything that looks like python code using pre-commit #1461

lint (black/pyflakes) everything that looks like python code using pre-commit #1461

graingert commented Oct 22, 2020 •

edited

Loading

graingert commented Oct 22, 2020 •

edited

Loading

adiroiban left a comment

adiroiban Oct 22, 2020

graingert Oct 22, 2020

adiroiban Oct 22, 2020

graingert Oct 22, 2020

adiroiban Oct 22, 2020

graingert Oct 22, 2020

adiroiban Oct 22, 2020

graingert Oct 22, 2020

adiroiban Oct 22, 2020

graingert Oct 22, 2020

adiroiban commented Oct 22, 2020

adiroiban left a comment

adiroiban Oct 22, 2020

rodrigc commented Jan 3, 2021

adiroiban commented Jan 3, 2021 •

edited

Loading

graingert commented Jan 3, 2021 •

edited

Loading

rodrigc commented Jan 3, 2021

lint (black/pyflakes) everything that looks like python code using pre-commit #1461

lint (black/pyflakes) everything that looks like python code using pre-commit #1461

Conversation

graingert commented Oct 22, 2020 • edited Loading

Contributor Checklist:

graingert commented Oct 22, 2020 • edited Loading

adiroiban left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adiroiban commented Oct 22, 2020

adiroiban left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rodrigc commented Jan 3, 2021

adiroiban commented Jan 3, 2021 • edited Loading

graingert commented Jan 3, 2021 • edited Loading

rodrigc commented Jan 3, 2021

graingert commented Oct 22, 2020 •

edited

Loading

graingert commented Oct 22, 2020 •

edited

Loading

adiroiban commented Jan 3, 2021 •

edited

Loading

graingert commented Jan 3, 2021 •

edited

Loading