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

zest.releaser plugin #162

Closed gforcada closed 7 years ago

gforcada commented 9 years ago

Temporal work in progress, I will try to finish it during the next days.

Fixes https://github.com/plone/plone.recipe.codeanalysis/issues/154

gforcada commented 9 years ago

@do3cc the more I think of the "add the code analysis status on the README file" suggestion the more difficult it gets: get the file, append/replace the code, formatting issues (no header? not rst? ...), not sure if it pays the effort.

Specially considering that this is something you run when about to release, so you shouldn't shoot on your foot while about to jump...

Opinions?

jensens commented 9 years ago

my opinion: writing a status.txt is perfectly fine, but modifying readme is imo too much

gforcada commented 9 years ago

@jensens I was wondering about that as well, just writing it to an extra file. But then I thought that this means having to take care of an extra file and appending it on the long description and the downside that being an extra file is not shown on the github/bitbucket/whatever main page of a repository...

do3cc commented 9 years ago

It was never a requirement from my side, just an idea. I like status.txt, it would of course be nice, if it was part of the docs for pypi.

gforcada commented 9 years ago

Then it just replaces status.txt every time and it's wired up on setup.py manually, header included.

That sound easier and more maintainable :-)

gforcada commented 8 years ago

Anyone willing to merge? :-)

gforcada commented 8 years ago

I just noticed that without #135 this is useless, as zest.releaser is creating a temporal clone of the repository, so no bin/code-analysis is available there nor you can do #135

gforcada commented 8 years ago

This is ready for review.

I decided to not save bin/code-analysis output on a file as there is too much complexity for little benefit. One can do that as a manual step before releasing.

mauritsvanrees commented 8 years ago

Technically: seems good to me. +1

But I have some thoughts.

I am wondering how we intend to use this. Will we add this to coredev 5 in the releaser part so this gets called when using the fullrelease command?

This is trying to run bin/code-analysis in the directory that has the setup.py of the package you are releasing, right? Can we check if this command is available before asking to run it? Or check for a config file? If it is not there, we could either silently accept this, or warn the user and ask if he wants to continue.

Or does it maybe make any sense as fallback to look for a code-analysis command in the same directory as the fullrelease script?

What I can imagine, is that a core Plone package has a buildout.cfg setup for p.r.codeanalysis, but that the person who makes the release has not run this buildout, as he is simply using the coredev buildout. Same is true for someone doing development on this package actually. It would be good to still run the code-analysis in this case. But when the defaults are no good match for this package, or no one has done the initial push to get this package in shape, then this is no good. Said differently, I am looking for the middle ground between the good of running the codeanalysis even when the release person has not run bin/buildout, and the bad of the recipe complaining about a package that is totally not ready for it. I am not sure there is such a middle ground.

The change to the README still says we offer the user to write it to a file status.rst.

As an aside, as alternative for registering this hook globally, it might be worthwhile for a to-be-released package to opt-in to this hook in its setup.cfg, a possibility that I was just recently reminded of. See https://github.com/zestsoftware/zest.releaser/issues/172 This can be a nice trick to have up your sleeve and be aware of. But for p.r.codeanalysis the global hook seems best to me.

gforcada commented 8 years ago

@mauritsvanrees thanks for the review and valuable feedback!

I will try to work them out during today or next days :-)

jensens commented 7 years ago

Is this still an issue?

gforcada commented 7 years ago

Most probably not as p.r.codeanalysis already installs a git hook, so you already see the output while zest.releaser is doing commits...