Skip to content
This repository has been archived by the owner on Mar 21, 2019. It is now read-only.

Add Flake8 support for tox test runner #13

Merged
merged 10 commits into from
Dec 22, 2017
Merged

Conversation

rjw57
Copy link
Member

@rjw57 rjw57 commented Dec 14, 2017

Add flake8 as a code-style check to tox test runner. Since CircleCI runs the default tox envs, this means that code-style should be enforced for each pull request.

Closes #12

@rjw57 rjw57 requested a review from a team December 14, 2017 13:53
@rjw57
Copy link
Member Author

rjw57 commented Dec 14, 2017

I hadn't noticed that I have some squashed flake8 errors set globally to reduce noise with our other projects. The project should now be flake8 clean.

.editorconfig Outdated
indent_style=space
indent_size=2
[*.py]
max_line_length=79
Copy link
Member

Choose a reason for hiding this comment

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

I'd vote for 119 line length. @msb ? @rjw57 ?

Copy link
Member Author

Choose a reason for hiding this comment

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

We would have to add configuration to flake8 to ignore line length since it enforces PEP8: https://www.python.org/dev/peps/pep-0008/#maximum-line-length

Copy link
Member Author

Choose a reason for hiding this comment

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

And I don't know off the top of my head if flake8 supports a configurable line length as opposed to just ignoring line length errors.

Copy link
Member Author

@rjw57 rjw57 Dec 15, 2017

Choose a reason for hiding this comment

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

And even if we make use of PEP8's wiggle room, to paraphrase Monty Python and the Holy Grail "99 is the maximum line length and the length of the maximum line shall be 99. 119 is right out!"

Some teams strongly prefer a longer line length. For code maintained exclusively or primarily by a team that can reach agreement on this issue, it is okay to increase the nominal line length from 80 to 100 characters (effectively increasing the maximum length to 99 characters), provided that comments and docstrings are still wrapped at 72 characters.

Copy link
Member Author

Choose a reason for hiding this comment

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

The consensus appears to be "99".

PEP 8 has the following to say about the subject of line length:

"Some teams strongly prefer a longer line length. For code maintained
exclusively or primarily by a team that can reach agreement on this
issue, it is okay to increase the nominal line length from 80 to 100
characters (effectively increasing the maximum length to 99 characters),
provided that comments and docstrings are still wrapped at 72
characters."

As discussed in uisautomation#13, we've settled on 99 as a compromise between
sticking to PEP8 and making use of modern screen widths.

Update the editorconfig and add a flake8 configuration file to the root
of the repo.
The line length silencing is now no-longer required because we allow a
longer line length.
@rjw57 rjw57 merged commit 063f87f into uisautomation:master Dec 22, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants