[Zope-CMF] GenericSetup: Apply profile dependencies only once

yuppie y.2015 at wcm-solutions.de
Wed Sep 23 16:56:06 CEST 2015


Hi Jens,

Jens W. Klein wrote:
> On 2015-09-22 12:30, yuppie wrote:
> [...]
>>> - pep8.  This fixes over 6000 pep8 errors... Most of them fixed with the
>>> autopep8 command line tool.  Small in scope yes, but due to all those
>>> errors a *very* large pull request.  All tests pass.
>>
>> -1
>>
>> I agree with the goal to try to respect pep8 rules and to use tools that
>> help doing this. But this is a massive reformatting that adds a lot of
>> noise if you use blame or similar techniques. And I use often diffs
>> between different versions to understand the history of the code.
>>
>> There might be a subset of pep8 rules that is already respected in most
>> parts of the code and where fixing the rest wouldn't add much noise.
> 
> I dont agree. The noise is one commit. Blame does not make sense without
> looking at the whole history anyway. So its one more diff in a whole series.

why would you look at the whole history? Blame tells you which revision
modified the lines you are interested in. So you can jump directly to
that revision. It's annoying if this revision is just a code cleanup.

> My only point is to not make code pep8 is to not affect other peoples
> branches/ open pull requests, because rebase/merge after any massive
> change is indeed lot of work.

This is not an issue with GenericSetup, but if you have several release
branches, it also makes it hard to port changes from one branch to an other.

I'm not completely against code cleanups, but I don't think touching
thousands of lines just to do massive automated cleanups is the right
way to do it.

Cheers,

	Yuppie



More information about the Zope-CMF mailing list