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

[Steffen Kamper]

Hi Jigal,

Am 29.03.2011 11:10, schrieb Jigal van Hemert:
>> 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.
>

this isn't hte case, core votes are required anyway to submit a patch, 
same for testing (verify).
+1 from core dev only signals that another vote is needed.
And as Karsten already pointed out: if a core dev votes at first place 
with +2 and another voter votes +1, the +2 is overwritten.
I'm not sure about this as i'm puzzled too, i had an issue where i 
posted +2 at first place, another voted +1, and overview stated +1 only.
At the end i look more to the review overview on upper left which shows 
clearly: was it tested? wwas it voted by core-dev? a.s.o.

> 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.
>

anyone now can submit, no problem - credits are all in the changelog. 
But be careful, some patches has to be applied to other branches as well.

>> 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.
>

np from my side - i view again, see the non-functional changes and give 
my vote again.

>> 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.
>

just wait a bit or ask the reviewers direct.

> 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)
>

i think that gerrit can't have a complete KI. It's only a tool showing 
all the action in a nice way. Sure, there are a lot of hidden and 
missing features, usability is not high standard, but to be honest, it's 
much better than only the news list ;)
And i didn't got much problems, some in the beginning, but mostly 
because faults on my side.

But i agree - we have to enhance the troubleshooting area in wiki.
Yesterday i learned a new thin and it should be a rule:

Whenever you start modifying files, create a branch!!!

I did the fault working on master only, pushing an issue, reset master, 
working on next issue - and suddenly i got dependencies where no 
dependencies were needed.

vg Steffen