search

collective / buildout.plonetest

Testing/development buildouts for Plone
6 stars 12 forks source link

Remove flake8-ignore setting #68

Closed thomasmassmann closed 4 years ago

thomasmassmann commented 4 years ago

This should be set in setup.cfg, tox.ini or .flake8. Unsetting in a custom config is not possible once set.

If you try to unset it later with e.g.

flake8-ignore =

it is still added to the list of flake8 options and written to the generated command:

'flake8-ignore': '',

This then overrides any settings in setup.cfg, tox.ini or .flake8.

idgserpro commented 4 years ago

@thomasmassmann we have to consider that there is a tendency to use black on Plone. See:

https://github.com/plone/Products.CMFPlone/issues/2754

Removing the E501, those who aren't closely monitored will end up correcting the code with a better line size than black allows.

@gforcada @mauritsvanrees opinions?

@thomasmassmann it might be good to open an issue in plone.recipe.codeanalysis so as not to overwrite the configuration, when we have flake8-ignore =

thomasmassmann commented 4 years ago

@idgserpro, I get your point. The problem is that the configurations are not merged. The flake8-ignore option is added as a command line argument to flake8, so that all settings in other configuration files are ignored. bobtemplates.plone e.g. uses buildout.plonetest, but also sets flake8 options in setup.cfg. They can not be used because of this hard setting here. And the E501 is set there as well.

Because so many buildouts extend buildout.plonetest I would recommend to remove this line here, instead of patching plone.recipe.codeanalysis which would mean that we have to override the setting in every single buildout configuration. Also, this setting was added here before it was possible to use the flake8 settings from setup.cfg etc.

idgserpro commented 4 years ago

@thomasmassmann I was thinking in packages that were not created with bobtemplates.plone or that were created on older versions of it.

idgserpro commented 4 years ago

@thomasmassmann this issue should happen with other options as well, not just with flake8-ignore. It should be possible to unset any option, to be able to use setup.cfg.