plone / bobtemplates.plone

Python Code Templates for Plone Projects with mr.bob
https://pypi.org/project/bobtemplates.plone/
GNU General Public License v2.0
24 stars 31 forks source link

Code analysis errors in newly generated addon and content_type template #405

Open idgserpro opened 5 years ago

idgserpro commented 5 years ago

To simulate:

plonecli create addon teste.teste
cd teste.teste
plonecli build
bin/flake8 src

src/teste/teste/tests/test_setup.py:3:1: I001 isort found an import in the wrong position
src/teste/teste/tests/test_robot.py:2:1: I001 isort found an import in the wrong position
src/teste/teste/locales/update.py:44:11: P202 format call uses missing keyword (excludes)
src/teste/teste/locales/update.py:44:11: P302 format call provides unused keyword (exclude)
src/teste/teste/locales/update.py:49:25: C812 missing trailing comma

plonecli: 1.1 bobtemplates.plone: 4.1.3

Is it possible to add this flake8 check to travis somehow? bin/flake8 gives 1 when it fails.

This happens in Plone 4.3 and 5.1.

erral commented 5 years ago

Travis is already configured to run the code-analysis part already provided by buildout

MrTango commented 5 years ago

please check if the failing tests are releated to the name of your add-on. We can not fix this, we have to stick to one order, if your add-on is named differently, it might need a different order, wich is easily done by isort.

idgserpro commented 5 years ago

please check if the failing tests are releated to the name of your add-on.

The isort yes. But changing the name to collective.newaddon gives:

src/collective/newaddon/locales/update.py:44:11: P202 format call uses missing keyword (excludes)
src/collective/newaddon/locales/update.py:44:11: P302 format call provides unused keyword (exclude)
src/collective/newaddon/locales/update.py:49:25: C812 missing trailing comma
idgserpro commented 5 years ago

Travis is already configured to run the code-analysis part already provided by buildout

I was thinking about flake8 running against a generated addon. It would catch the errors above.

For example: the errors from above were added by https://github.com/plone/bobtemplates.plone/commit/99280fc58b0a4b02291fdc1a859b158ebba8287e

But the build didn't fail: https://travis-ci.org/plone/bobtemplates.plone/builds/568755503?utm_source=github_status&utm_medium=notification

idgserpro commented 5 years ago

Just by running a content_type template with all default options gives errors as well:

src/collective/myaddon/tests/test_ct_todo_task.py:43:9: F841 local variable 'obj' is assigned to but never used
src/collective/myaddon/tests/test_ct_todo_task.py:46:5: E303 too many blank lines (2)
src/collective/myaddon/tests/test_ct_todo_task.py:55:9: E303 too many blank lines (2)
src/collective/myaddon/tests/test_ct_todo_task.py:67:59: C812 missing trailing comma
src/collective/myaddon/tests/test_ct_todo_task.py:79:10: E121 continuation line under-indented for hanging indent
src/collective/myaddon/tests/test_ct_todo_task.py:88:71: C812 missing trailing comma
src/collective/myaddon/content/todo_task.py:2:1: F401 'plone.dexterity.content.Container' imported but unused
MrTango commented 5 years ago

you are right, we should check that we run the linting in the skeleton tests too. fir the concrete fixes, PR's are very welcome ;). I have a lot on my list already, also working on upgradesteps template and some other things.

idgserpro commented 5 years ago

PR's are very welcome

We know... Unfortunately, bobtemplates.plone is on plone namespace and we can't sign the contributor's agreement because of some internal legal reasons.

We know how hard it is to handle all these issues. When we can, we try to add PR's instead of just opening issues (like in https://github.com/collective/buildout.plonetest) but only on collective namespace.

MrTango commented 5 years ago

I'm sorry to hear that. But in case you have ideas you want to realize as a plonecli/bobtemplate you can do that too as a seperate package and put it in collective. bobtemplates packages can register there templates for bobtemplates and plonecli usage, swee here: https://pypi.org/project/plonecli/#register-your-bobtemplates-package-for-plonecli.

balavec commented 4 years ago

Not sure if this is the right place ... but it is definitely related:

I also ran into issues with flake8 because of C812 missing trailing comma. I understand why. I am also using black to format the python code. Obviously they have some conventions of their own like Q000 Remove bad quotes, so my question here: what is the policy about using black? And/or maybe also add .flake8 config file?

MrTango commented 4 years ago

@balavec in general what you can do is add more excludes to the flake8 linter, for that you don't need an extra config file, you can do that in the setup.cfg of the package. It might be that we should add this to the template file of the addon template in bobtemplate.plone so that new packages are created with this setting. If this is the case, please check that, then let's add in the setup.cfg.bob in the addon template. PR welcome.