[TYPO3-core] Gerrit and new review policy for TYPO3 v4 Core

[Jigal van Hemert]

Hi,

On 28-3-2011 23:12, Steffen Kamper wrote:
> beside all the bureaucracy there is no difference to the system we had
> before.

Bureaucracy isn't a very positive thing :-(
In theory the system is equal, but not in real life (see below).

> The core devs are responable for the submits and they should do
> it with care. Gerrit now gives the real advantage to count votes very
> clearly, before you had to scan the complete thread.

Which wasn't a big problem. If the first core dev with a positive vote 
didn't do that (or the patch owner if that was a core dev) then others 
would certainly notice it.
News readers like Thunderbird allow you to tag posts and threads. This 
is comparable with the watched changes feature of Gerrit. So still no 
advantage here.

> The commit rules didn't changed, only the system did. And this is a big
> help.

There are some subtle differences and they already start to become a 
problem:
Previously a patch needed a +1 from a core dev and a +1 from anyone.
Now you need a +2 from a core dev and a +1 from anyone.

Unfortunately core devs can also give +1s and do this frequently. This 
is really not very helpful, because with 10 +1s the patch can still not 
be submitted.

Also, the rule was that the first core dev involved (either the patch 
owner if that was a core dev or the first core dev to give a +1) would 
take care of committing.
Now you have the option to give your vote (Publish) and not commit 
(Submit) the patch.

> Of course a new patchset resets the voting level, which is indeed fine.

Even for non-functional changes (CGL, commit message, etc.) the voting 
is reset. Gerrit cannot decide, but this is not fine in all cases IMO.

> The already voters should review the new patchset again, this should be
> easy as they already did it.

Even in the cases where it's useless (and responsible people such as 
core devs should be capable of deciding that) to review it again the 
votes are needed in Gerrit. Of course you can Submit the patch to the 
repository, but this is not done most of the time.

Gerrit could be a decent system for reviewing, but in its current form 
it has a couple of disadvantages:

- the UI has usability issues in several areas
- the voting system gives too much room for leaving responsibility to others
- resetting votes for new patch sets in all cases requires too much work 
for reviewers and puts a break on development
- the problems with cherry-picking/merging patches into the repository 
require manual interference, new patch sets and repetitive reviewing; 
this puts a break on development

The last point is worse for v4 development than for v5 development, 
because for historical reasons v4 classes are much bigger and chances 
are higher that something has changed between making a patch set and 
submitting it. (Even applying an svn patch to your local working copy 
was way more intelligent than Gerrits cherry picking operation)

-- 
Kind regards / met vriendelijke groet,

Jigal van Hemert.