[TYPO3-core] Patch Sets in Gerrit

[Ernesto Baschny [cron IT]]

Hi,

it is very convenient to upload new patchsets to existing reviews on
Gerrit. One disadvantage is that the voting score is "cleared". But that
hasn't been a problem yet.

One problem I have with reviews that contain >10 Patchset is when new
patchsets are not described. I would encourage people to write a few
lines *why* they are sending a new patchset in the comments after
uploading it.

For example:

https://review.typo3.org/#/c/11804/

Jigal made some note on Patchset 19. Then came patchset 20 and 21 (Jo
and Georg), without any comment. Was Jigal's point considered?

Other prominent example is the Responsive Image rendering:

https://review.typo3.org/#/c/22052/

We are already at patchset 29. Of course I can often do a "diff" between
patchsets to see "what has changed", but it helps focus on the important
aspects if you have a sentence describing what you did.

Here's an example where I commented new patchesets after upload:

https://review.typo3.org/#/c/24187/

(see #4, #5).

Often a new patchset is uploaded just to "rebase" to the current state
of master (and resolving conflicts etc). This is something worth
mentioning in the notes and help people understand why there is a new
patchset. And if you are rebasing, it probably means you have some
interest in this particular patch, maybe explaining it a bit more at the
same time helps other occasional reviewers undestand why they should
review it.

Also please remember that rebasing a patchset will most probably make it
difficult (if not impossible) to compare it to a previous patchset, if
there has been other changes in between in the same file. So reviewers
that are using the "Diff Patchsets" tool to incrementally review newer
patchsets will have trouble with it. Consider uploading a rebased
patchset *only* in the situation when there is a conflict while rebasing
or the patchset is already really old.

What do you think? Other opinions? Nobody stumbled over these problems yet?

Kind regards,
Ernesto