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

Maurits van Rees m.van.rees at zestsoftware.nl
Tue Sep 22 21:55:14 CEST 2015


Op 22-09-15 om 12:30 schreef yuppie:
> Hi Maurits,
>
> Maurits van Rees wrote:
>> Meanwhile, I have added two more pull requests, far smaller in scope:
>>
>> - Add 'unsetLastVersionForProfile' method to portal_setup. This removes
>> the profile id from the profile upgrade versions.
>
> +1

Adding 'purgeProfileVersions' is also on my wish list now, which is 
really simple:

     portal_setup._profile_upgrade_versions = {}

There were a few problems in Plone due to my change with the dependency 
strategies.  I found that those were caused by importing the base Plone 
profile (so no extension profile), so this ran in purge mode, which 
meant several settings of add-ons were overwritten.  Only a problem you 
really ever encounter in test code that tries to do to much.

Purging the profile versions from portal setup helped solve this.  And I 
would rather call an official method than accessing the private 
_profile_upgrade_versions from within Plone code.

>> - 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 can imagine your concern about reduced value for 'blame'.  You can 
still checkout a previous version and do the git blame there, but it is 
more of a hassle, I agree.  It is personal preference whether this 
reduced blame value weighs more heavily than the reduced frustration of 
working with ugly looking code.

But actually, it is not that bad I think.  When I look at the top level 
files at how many lines of them are to 'blame' on my pep8 change, I get 
this table.  Not sure if this gets across nice in email:

Filename	Blame	Lines	Percentage
__init__.py	5	55	9%
components.py	28	559	5%
content.py	25	417	6%
context.py	162	723	22%
differ.py	64	196	33%
events.py	0	55	0%
exceptions.py	0	21	0%
interfaces.py	63	847	7%
metadata.py	20	77	26%
permissions.py	0	16	0%
registry.py	44	746	6%
rolemap.py	44	219	20%
testing.py	0	178	0%
tool.py		105	1426	7%
upgrade.py	14	274	5%
utils.py	49	927	5%
zcml.py		20	372	5%

So the file with the biggest percentage of lines changed, is differ.py 
with 33 percent.  We have context.py, metadata.py and rolemap.py between 
20 and 26 percent.  The rest is below 10 percent.
The biggest and most central one, tool.py, has 7 percent of its lines 
changed.

In the tests directory things are very different, given that there are 
about 5000 pep8 errors there.  git itself says for three files that it 
completely rewrote them:

  rewrite Products/GenericSetup/tests/test_differ.py (66%)
  rewrite Products/GenericSetup/tests/test_registry.py (78%)
  rewrite Products/GenericSetup/tests/test_rolemap.py (64%)

But I would say for the tests a 'git blame' is less needed.


-- 
Maurits van Rees: http://maurits.vanrees.org/
Zest Software: http://zestsoftware.nl



More information about the Zope-CMF mailing list