Re: Pull-Request Crusade [was Re: [Products.Archetypes] Correctly...]

Hi Dylan!

Thanks for your input. Below some further thought of mine about this.

On 2014-05-14 03:09, Dylan Jay wrote:
> On 13 May 2014, at 5:30 pm, Jens W. Klein <jens-/[email protected]> wrote:
>
>> On 2014-05-13 08:44, Jean Jordaan wrote:
>>> Hi Jens
>>>
>>> Awesome old-issue crusade that you're on B-)
>>
>> Well, yeah. Its more a pull-request crusade. Its the hell out there. And
>> thats only pull-requests, i didnt looked at issues overall.
>>
>> There are 141 pull requests open (roughly 20 closed so far) at
>> https://github.com/organizations/plone/dashboard/pulls/public
>>
>> I try to close what I can. For some topics its just not possible for me,
>> it needs the original authors to decide if it makes sense or not.
>>
>> But many are simple fixes where just a changelog entry is missing or
>> where the auto-merge doesnt work any longer.
>>
>> So help appreciated - and if its just a short comment from people deeper
>> into the package.
>>
>> I think looking at pull-requests (and issues) is also a good and needed
>> sprint topic - imo it must be a topic on every future Plone Sprint.
>>
>> As a contributor/requester it must be completly disapointing: One has
>
> As someone who some of those pull request belong to, it is disappointing.
>
>
>> some kind of problem, solves it, makes a pull request and theres no
>> response (not in all cases, but there are several). Then it hangs for
>> years!
>>
>> I dont think we shall do this as community and need a process here.
>> First positive guidance for newbies - tell them to sign contributor
>> agreement, that we want a chnage log entry, maybe about tests and pep8,
>> how to add changes to a pull request (i know, just commit to branch, but
>> thats not clear).
>
> It's not an easy thing to fix. Who's job is it to review pull requests? the FWT? Do they do too much already?
> It's not clear whose responsibility it is. The best people to review code are the people already writing the most code so we can't expect them to do more.
> Without someone clearly responsible you get -> http://en.wikipedia.org/wiki/Diffusion_of_responsibility
> As a contributor it's made worse by not knowing who to ask. I tend to look at who did the last few commits to a package and @ ask them for comment. Not really fair and since they aren't official responsible they may just choose to ignore the request without consequence.

> Perhaps the best way to solve this is to do what you are doing Jens. Have someone, who isn't the contributor, find and hassle people to review open pull requests. Someone who knows the right people to ask and doesn't overload the busy people. Should that be the release managers job?
>
> The consequence of not doing something is that Plone can be a very demotivating place to contribute to and this is at a time where we need more contributions.

In my opinion both, FWT and Release Manager, are only responsible for 
orphaned core packages. But they dont care (expect personal interests - 
sorry, but thats what I observed). Focus is primary only on the new 
things. This is indeed a problem and we need to discuss how to make it 
better.

I'am sure its just the lack of time. Eric is doing a awesome job, but he 
is already slighlty overloaded (thinking about the way too complex 
release process). So its not an option to simply say: "Your job release 
manager."

Framework Team is where this role is placed better - at least for all 
packages that are or were at some point delivered as part of Plone (Zope 
and beyond excluded). If its too much for the current members 
(time-wise) we may need to think about extending the circle.

But what about those in githubs plone organization outside of that 
scope? I think if IP is transfered to Plone Foundation it has a certain 
amount of responsibility here too.

So here an opinion of FWT is needed.

And if this is part of responsibility of FWT there must be one or two 
persons taking time an every N days/weeks interval looking over pull 
request and issues without any progress.

We need a transparent process how to deal with pull requests, taking 
contributors seriously (i.e. not just rejecting because of missing tests 
or pep8 - sometimes in old code this does not make sense).

Idea of a process:

- add a section 'contributing' to the readme where missing. We need to
   dicuss how this should look like. It may be different for old
   packages with now/low testing and new which are core. Same for pep8.
   In older packages its often better to just fix that line than
   reformatting everything.

- Minor changes (i.e. documentation, translations, typos, ..): Check
   if change log entry is there, just merge.

- Bigger changes and you know whats going on and everythings fine:
   Merge

- If theres a maintainer and now answer after 2-3 weeks: poke a bit,
   try get a comment, try to comment yourself. Follow up later.

- No maintainer or maintainer doent answer after 2-3 weeks:

   - Mark orphaned.
   - Comment yourself, find a reviewer.
   - Find a new maintainer, poke last contributors, ask the submitter of
     the pull-request. Write at developer list.

- dont keep pull-request open forever. Come to a decision at some
   point. Merge or close giving a clear reason. But first give the
   requester a chance to make the solution better. If theres no repsonse
   after 2-3 weeks: close.

Did i miss something important?

regards Jens

>>
>> Anyway, I'll try to go ahead a bit here. Depends also a bit on my time.
>>
>> So as said: Help wanted and if its one picks a random pull request and
>> looks at it and write a short comment. Not much effort, great effect!
>>
>> regards Jens
>>


-- 
Klein & Partner KG, member of BlueDynamics Alliance


------------------------------------------------------------------------------
"Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
Instantly run your Selenium tests across 300+ browser/OS combos.
Get unparalleled scalability from the best Selenium testing platform available
Simple to use. Nothing to install. Get started now for free."
http://p.sf.net/sfu/SauceLabs