[hibernate-dev] Retrospective on "Pull Requests": a waste of time?

[Sanne Grinovero]

I think we all agree that the current process is very good: don't take
this too seriously. I'm merely trying to identify how we can improve
on efficiency even more, and also I am coming to the conclusion that
often I'm the bottleneck myself so I'm trying to figure out what's
preventing me to streamline more *and* better code.

The question is not if I want a quick merge or a review: the world is
not black and white, my point is that when we find something that
could potentially be improved during a pull request review we have to
consider additional cost factors and make a responsible decision
between all options. This does not rule out a push back or demands for
improvements at all! Simply put, let's be aware that we have also the
option to open follow up JIRAs, and that in several cases that might
be the best thing to do.

Hardy I'm surprised that you seem adverse on this proposal, as
actually you helped me several times by applying such a strategy: I
found it very effective and am grateful as in those cases I simply had
no chance to work on the requested improvements in a timely matter..
better to log them in JIRA than to let them fall through the cracks
right? So my idea was to bring this up on the mailing list, as a
general suggestion - WHEN FIT - of course.

Let's try to make sure we don't stick with an unflexible process but
focus on progress; if you need a rule of thumb, mine is this one: "if
it makes progress, merge". (That it doesn't break anything is I hope a
self-speaking hard rule).

Any change on the codebase which results in a better code than what it
would be without the patch should be merged ASAP, even if there are
further improvement possible. Of course both pusher and reviewer have
to agree that the code base is indeed _better_ whatever that means..
case by case, and if there is no agreement the merge is rightfully
blocked or at least rediscussed.

Examples:

 - your metadata API is not complete, yet I merged two chunks of
progress, as I considered them solidly good stuff. You don't disagree
do you?

 - I'm upgrading Infinispan and get side-tracked by a test which is
not cleaning up properly, so I fix the cleanup as I need it, but
technically this already is a costly context switch for me. You then
identify more things that could be improved on that same test - and
rightfully - but I'm having a hard time debugging totally different
things. I think this is very reasonable to put on a "TODO" list,
rather than having me context switch .. I guess you were not aware
that I wasn't working on that test, and hope you see the point now?
Also considering you looked at it, and I'm not following the thing, I
believe as a team we can act more efficiently if you jump on the
improvement yourself - as you indeed did and I think it was a good
team work.

Now the question is if yesterday we "violated our rules" or have been
wisely using our tools?
I think we have been wise, as that's a success story and would like
that to happen more often: your help gave me the opportunity to finish
my debugging and bring home some nice progress.

On 10 July 2013 10:09, Hardy Ferentschik <hardy at hibernate.org> wrote:
> As much as I enjoy this discussion, right now we have 0 open pull request in Search.
> So where is the problem?

There isn't a problem, as we're all being relatively flexible and it's
working out quite well. But to achieve that, we've gradually
redefining the process a bit to better match our needs. I think we can
still improve on it, I just want to make sure we're clear that the
rules are flexible and should not constrain you from making practical
adjustments more often. Personally I can definitely use some help on
avoiding context switching, I admit I'm struggling and need you on
this.

Cheers,
Sanne