Re: Considering Gerrit for CFs - Mailing list pgsql-hackers

On Fri, Feb 8, 2013 at 1:32 AM, Josh Berkus <josh@agliodbs.com> wrote:
> Folks,
>
> First, thanks for the serious discussion of this.
>
>>> There are obvious tooling gaps (aren't there always?), but I don't
>>> really see the model as broken, and I don't think I've been around
>>> pgsql-hackers exclusively or extensively enough to have developed
>>> Stockholm syndrome.
>
> I don't see the model as broken either.  Just the tooling, which is why
> I'm looking at tooling.  As in, I'm looking for better tooling in order

Yet you are suggesting tooling that requires a change in the model?


> to solve two problems:
>
> 1. maximize the efficiency of existing reviewer time
>
> 2. make tooling not be an obstacle to getting new reviewers

I think you are missing a fundamental part in this - which is "0.
don't negatively affect the efficiency of existing committer time".
I'm not saying it necessarily does (though I think it does, but that's
not a proven point), but that has to be a pretty high priority.


> Of these two, (2) is actually the more critical. We have been losing,
> not gaining, active committers and reviewers for the last couple years.
>  Clearly "do more of what we've been doing" is a losing strategy.   We
> need to be sucessfully moving people up the contributor chain if we're
> ever going to get out of this "not enough reviewers" hole.

Agreed. But do you have any actual proof that the problem is in "we
loose reviewers because we're relying on email"?


> I agree that tooling is a minority of this, but tooling is also the
> easiest thing to change (compared with project organization), so that's
> what I'm tackling first.  Expect a discussion on the people aspects at
> the developer meeting.

It would probably be a good thing to discuss the tooling there, too.


>> Personally, I find the most annoying thing with the current process
>> being when reviewers post their reviews as a completely separate
>> thread, instead of replying in the original thread. This causes
>> context switches when parsing things, because now you have to jump
>> back and forth between the CF app and your mail reader. But it's still
>> only on the annoyance side, I think the process in general is not
>> broken. (That said, I *have* been on the inside a long time, *and* I
>> live in Stockholm, so I might well have that syndrome)
>
> So, look at this from the perspective of a casual reviewer, say at a PUG
> reviewfest.  Instructions to new reviewer:
>
> 1. find the feature you want to review on the CF app.
>
> 2. Click the link to the mailing list archives.
>
> 3. Click all the way through the list thread to make sure there isn't a
> later version of the patch.

Supposedly the latest version should always be listed in the CF app.
The fact that this is a manual step is a problem, that could probably
be fixed quite easily.


> 4. Download the patch.   Hopefully it's not mangled by the archives
> (this has gotten much better than it was last year)

Yes, several things have been done to make this work better. There
shouldn't be any issues at all with it now - and if there are, we are
in a much better position to fix them.


> 5. Apply the patch to HEAD clone.
>
> 6. Do actual reviewing/testing.
>
> 7. Write an email review.
>
> 8. Send it to pgsql-hackers
> 8.a. this requires you to be subscribed to pgsql-hackers.

No, it does not. It will get caught in the moderation queue and get
slightly delayed if you're not, but it works perfectly fine.

And if we were to use another system, you'd still have to sign up for
that one, so it's not really that big a problem.


> 9. wait for the email to show up in the archives.

You do realize this currently counts within seconds or something like
that, rather than the 15+ minutes it used to be? And the fact is, you
don't actually have to wait for it.


> 10. create a review comment in the CF app.
> 10.a. this requires you to be signed up for a community account
> 10.b. sign up for one now
> 10.c. wait for it to be approved

Huh? There is no approval process for community accounts. There is a
verification step, of course, but any system would have that.


> 11. link the review comment to the messageID
>
> 12. change status of the patch
>
> This is a few too many steps, and certainly appears completely broken to
> any newcomer.

I agree it's way too many step. Several of those can certainly be made
more efficient now that we have a more sane archives, well within the
scope of the current system.


--Magnus HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/