Example of increased strictness on linting

[agjohnson]

This increases prospector base profile to medium strictness, enables pep257,
and adds some additional pylint rules to filter out docstring related failures.
This limits docstrings to methods/classes less than 10 lines, which should be
readable already. Docstrings wouldn't hurt in this case of course, but we won't
require them on everything.

[agjohnson]

Only 1072 errors, 🍰

[agjohnson]

Down to 983 failures with more lenient rules around docstring required construct length. This number is inflated as it includes the fixes to projects/ in #1580, which would bring this down to ~800 failures.

[ericholscher]

Looks like this is mostly up to date with where we are currently. I've repushed it w/ a merge from master, to see where we are :)

[agjohnson]

There are a couple of new additions that we should target here -- a recent PEP now specifies that class should not have a space between class definition in docstring, conflicting with an older PEP that states otherwise.

I've been using this profile locally, by overriding the default with my vim settings. That's a great way to work on this incrementally until we're ready to take more chunks off.

[ericholscher]

Adding a few exceptions and turning pep257 off gets us down to ~200 mostly
valid errors (unused imports & arguments, etc)

On Fri, Jan 15, 2016 at 11:36 AM, Anthony [email protected] wrote:

There are a couple of new additions that we should target here -- a recent
PEP now specifies that class should not have a space between class
definition in docstring, conflicting with an older PEP that states
otherwise.

I've been using this profile locally, by overriding the default with my
vim settings. That's a great way to work on this incrementally until we're
ready to take more chunks off.

—
Reply to this email directly or view it on GitHub
#1581 (comment)
.

Eric Holscher
Maker of the internet residing in Portland, Oregon
http://ericholscher.com

[ericholscher]

Would be worth trying to get this up to medium, at least for the unused import checks -- I generally delete them when I find them, but would be good to have in CI.

[agjohnson]

I'd rather keep pep257 and attack this incrementally, pep257 has useful requirements.

A better option is to add the pylint options to the prospector config manually, starting with missing imports.

[agjohnson]

Okay, I'm making this a thing. I've added a secondary file and a secondary pass to prospector. The secondary pass uses path exclusion, so we can slowly bring in paths during cleanup.

[ericholscher]

Could probably use a bit of context in the prospector-more file as to what it's doing and why it's there, but looks like a good way to get this PR merged and on a path forward :)

[ericholscher]

Should put a comment here or something inviting folks to remove a directory and fix the errors going forward, perhaps.

[agjohnson]

This would be more visible as an issue, i think there is already one open.

[ericholscher]

This seems like a change we don't want in this PR?

[agjohnson]

I lumped this in with cleanup on this file, settings.postgres is long gone.

[ericholscher]

@agjohnson this ready to merge?