Using Gerrit for code review in KDE

[Eike Hein]

On 13.09.2014 21:10, Kevin Krammer wrote:
> So your suggestion would be to not have +2 but a policy of some sort that only
> the +1 of the maintainer, if there is an active one, is considered as "go
> ahead"?

Basically my thinking is roughly this: It actually happens
extremely rarely in practice that something gets committed
that needs to be reverted because it's actively objectiona-
ble. As Ivan pointed out, few of us will ever commit any-
thing if we're not confident it would meet with the approval
of our peers. We generally do a good job with seeking out
the opinion and approval of those in the know.

Yes, this requires a lot of trust - but we've always known
this; broad, equal write access requires just as much trust.
That's why we have a process for getting developer access
that tries to make sure those getting access have been
around long enough to understand the etiquette and can be
reasonably worked with; it's why getting developer access
involves peer review by other developers. The idea is that
trust can flow from this - when someone has a KDE developer
account you want to be able to rely on the fact that they
should have that account.

Having a +2 and restricting it to maintainers says we can't
trust KDE developer account holders. If this were actually
true, I think it would imply our process is broken on the
other end. The Manifesto is written in the spirit of rely-
ing on this trust mechanic.

But as said, I don't think it's actually true. I think we
*can* trust KDE developer account holders, and that's why
we don't need an extra safety net in the form of having a
+2 and restricting it to maintainers.

It's biasing the system in the wrong direction, and tries
to plan for an eventuality that happens extremely rarely
(and we have other safety nets: test suites, CI, beta
releases, ...), at the cost of making maintainer succession
more difficult down the road.

Maintainers should always think about the maintainers who
will one day replace them and make sure they can.


> If I brainstorm about alternatives I find:
>
> * let maintainers have -2 as pro-active variation of reverting

That still requires flagging people as maintainers in a
DB, though ...

We already flag maintainers on projects.kde.org, which as
mentioned I think was a mistake, but it mainly came about
for logistical reasons, not for security reasons. I think
we need to avoid using this as a precedent to entrench
maintainers elsewhere ACL-wise.


> * build social ettiquette to always wait for the maintainer's +1 but at most
> for a certain grace period, e.g. one week

I think this etiquette basically already exists in prac-
tice.

If I write a patch that touched complicated code in plasma-
framework that I know Marco knows much better than I do,
I'm not going to commit it without him having weighed in
on it. If Marco's on vacation I'll at least make sure
someone else thought through the problem and came to the
same conclusion as I did.

I fixed some bugs in KIO lately around file previews, and
struck up an in-depth conversation with David about it and
made sure I got his review and input for the changes I
needed.

And so on ... all without requiring a +2.


> * have everyone get +2 and use etiquette to do that only if there is already
> strong agreement or the grace period has passed

Seems best to me, assuming we can't change Gerrit to
remove the distinction entirely.


> Cheers,
> Kevin

Cheers,
Eike