search

plone / plone.recipe.codeanalysis

provides static code analysis for Buildout-based Python projects, including flake8, JSHint, CSS Lint, and other code checks
https://pypi.org/project/plone.recipe.codeanalysis/
11 stars 5 forks source link

Add two new recommended flake8 extensions #183

Closed gforcada closed 8 years ago

gforcada commented 8 years ago

Two extensions that enforce something that our style guide already claims, see http://docs.plone.org/develop/styleguide/python.html#breaking-lines

flake8-commas https://pypi.python.org/pypi/flake8-commas

Enforces method calls, lists and dicts to have a trailing comma on the last element.

flake8_strict https://pypi.python.org/pypi/flake8_strict/0.1.1

Forbids the first argument to be on the same line if it's a multiline statement. Also checks for missing comma on last argument

hvelarde commented 8 years ago

I think is pretty important to dedicate a section on the documentation to mention which are the recommended plugins, what do they do and which original option the supersede; I found myself the other day trying to guess that while updating plone recipe.codeanalysis on an add-on.

jensens commented 8 years ago

@hvelarde how would you extend https://github.com/plone/plone.recipe.codeanalysis#recommended-extra ?

gforcada commented 8 years ago

@hvelarde @jensens as I already wrote on the README's section my idea is to add only flake8 plugins that fit our agreed style guide, nothing more (well some helper thingies like deprecations is always nice).

hvelarde commented 8 years ago

@jensens I started using something like this: https://github.com/collective/collective.fingerpointing/blob/master/buildout.cfg#L17-L25

@gforcada your missing a line on the change log.

gforcada commented 8 years ago

@hvelarde I know, it's still not ready to be merged anyway.

gforcada commented 8 years ago

@jensens sorry but please take a second or two before merging pull requests right and left, @hvelarde already pointed out that it's missing a changelog entry and I also stated that is not ready to be merged anyway...

I appreciated a lot all your effort and time that you put in plone, but blindly merging pull requests is not always a way to improve things, but to cause problems... There's no need to rush :-)

jensens commented 8 years ago

@gforcada Sorry, but you set it to "24 status: ready" - if there is work to do I'd expect a "22 status: in-progress"

do3cc commented 8 years ago

Sorry @jensens, but 24 status ready can't mean, merge blindly. But this is what you did. Maybe status 24 is not needed in the collective, since those who can choose that tag can also just merge the code directly.

gforcada commented 8 years ago

@jensens well, right, that was my fault indeed, but merging blindly is not either good, so half me half you :-)

jensens commented 8 years ago

@gforcada ok, I'am ok with half-half ... I just saw there is a commit after the complains and did quickly recognize doc-enhancements. And w/o deeper looking into it I assumed it addresses @hvelarde complains.