Skip to content

Conversation

hukkin
Copy link

@hukkin hukkin commented Jan 26, 2020

  • Edited tox.ini:
    • Format every python file with black (pinned to version 19.10b0)
    • Use line length 79 for black (this is the flake8 default).
  • Ran black --line-length 79 . to auto-format every python file

This automatically fixes flake8 errors of types E111,E114,E226,E231,E266,E302,E305,E502,W391.

Possible closes #171

@coveralls
Copy link

coveralls commented Jan 26, 2020

Coverage Status

Coverage decreased (-0.05%) to 96.989% when pulling f757a23 on hukkinj1:black into c00a19a on warner:master.

@hukkin hukkin requested a review from tomato42 January 26, 2020 22:19
@hukkin
Copy link
Author

hukkin commented Jan 27, 2020

Two questions @tomato42:

  1. You mentioned in the issue a line length of 80. I used 79 here because that is the flake8 setting currently in use. I can change to 80 chars if you want?
  2. I set the black configuration (i.e. the altered line length) as a CLI arg in tox.ini. An alternative is creating a pyproject.toml file and adding the configuration there. Downside: one more conf file, upside: the conf is applied when calling black outside tox.ini (e.g. IDE autoformatter, pre-commit hooks etc.).

# E302: expected 2 blank lines, found 1
# E305: expected 2 blank lines after class or function definition, found 1
# E203: whitespace before ':' (this needs to be ignored for black compatibility)
# E501: line too long
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't we be able to remove this one too?

Copy link
Author

Choose a reason for hiding this comment

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

Theres a bunch of lines like

_b = 0x051953EB9618E1C9A1F929A21A0B68540EEA2DA725B99B315F3B8B489918EF109E156193951EC7E937B1652C0BD3BB1BF073573DF883D2C34F1EF451FD46B503F00

which is something black will not fix. These need to be manually sliced or we have to add # noqa: E501 comments on these lines (personally would prefer the latter). This could also be a separate MR?

Copy link
Member

Choose a reason for hiding this comment

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

those can be translated to int("0x0519...", 16) this is the approach we are already using for a lot of constants like this

I guess we can do it as a separate PR...

Copy link
Member

Choose a reason for hiding this comment

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

moved to #187

Copy link
Member

@tomato42 tomato42 left a comment

Choose a reason for hiding this comment

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

why the length limit is excluded in pylint?

@hukkin
Copy link
Author

hukkin commented Jan 27, 2020

Do you perhaps mean flake8? Seems to me that this project is not using pylint.

That check is ignored due to lines like the one mentioned here #186 (comment)

Autoformatter wont be able to fix these.

@tomato42
Copy link
Member

ok, let's move E501 to a separate issue

@tomato42 tomato42 merged commit 333ee3f into tlsfuzzer:master Jan 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

unify formatting between modules
4 participants