[Framework-Team] Reviewing PLIPS sux0rs | Github growing pains

Elizabeth Leddy elizabeth.leddy at gmail.com
Wed Jan 18 23:50:01 UTC 2012

Yo FWT peeps - 

I just finished reviewing a plip, the first time not in svn and I think there are some things that to happen for me to actually review more plips and not be intensely frustrated. I suspect anyone who reviewed plips already would agree with some if not all of these. People won't do reviews if they are frustrated and/or it takes freaking forever. 

These complaints are mostly rooted in the new github setup. I know how *I* use github and I hear there are a lot of reasons why we don't use it differently for plip reviews. I am not qualified to make suggestions because my head spins with all the arguments back and forth, I was hoping to just complain and then have you guys tell me to either can it or maybe we can work out something resembling sanity. 

Forking Mad: People love forking!
1. When people fork, it means that I have to delete my src/Plone.CMFPlone folders and any other folder that's now elsewhere, then rerun the long arduous process of downloading that honkeriffic package. (Grr).  We need a best practice here. Would branch the coredev buildout for each PLIP solve this problem? How can we get people to stay in the plone namespace on github? Can/should we require that namespace as part of the condition of getting into core?
2. I don't know where to find code commits for review in a sane way. Looking on the file system is nutty for large packages. I just spent 15 minutes trying to find a changeset by following forks, only to see that the only line added to the CMFCore was plone.packagename in yet another fork of someone else. (Grr)

3. Documentation is confusing AGAIN. It is tradition to use README in gihub to provide documentation. Should we be encouraging that with PLIP updates and packages? I personally like it, but then we have yet another place to have documentation. Also, when people have forked or are hosting in their account instead of under plone, holy whoah the docs could disappear. And what about collective? Can we set on a standard for at least one place for all new documentation? (Moar Grr)

4. I personally want to comment line by line in the code and did on this last review. This is SO much easier than putting reports in the text reviews, but then the other code reviewers need to look in the same place. Can we all agree to switch to commenting on commits and lines in github at the source instead of the review.txt file? 
5. Filing bugs. If we are in github and the package is forked, it seems like we should file the bug there. Or do we keep it in trac? I don't know what the current philosophy is. Maybe we just listen to the devs? I just want them to 1) get the bug and 2) fix it. 

6. Can we make directories in the plips to make autocomplete easier? Maybe having a directory for each plip that then has everything about that plip? My only concern is making sure the cfgs still run. The current directory layout makes my eyes bleed after so many plips.

I think at minimum, we need to have the PLIP implementors help us by *clearly* listing *urls* to 
 - All documentation, which should be at a url (should it always be in collective docs? should human docs go there and sphinx docs elsewhere? What about upgrade notes?)
 - Which bug tracker they want us to use (that or we tell them, but I don't think that will work)
 - A URL which directs us to all packages that have been changed.

On our end, It would be nice to provide a guide that tells the implementors...
 - all docs of type x go here, all docs of type y go here, all plips require docs of type X & Y
 - fork/branch code like this
 - submit for review with the urls above in implementation notes (plipXXX-XXX.txt)

This way we don't have a guessing game on both ends and additionally it get's way easier for people to add plips. I'm happy to do the legwork of documenting if I can get some consensus on where to put things and the best way to address these problems. 


More information about the Framework-Team mailing list