On Aug 30, 2012, at 9:59 AM, Antoine Musso hashar+wmf@free.fr wrote:
It is also a good practice to add a cover message on the new patchset to explain what it changes compared to the previous one.
Yes, very important. If you submit a patch set, please do leave a quick comment explaining what you changed. I personally like to use bullet points for those comments like:
* Rebased
or
* Addressed comments from John. * Removed redundant code in foo.js.
PS3: rebased on latest master PS4: fix the, now obsolete, function() call
Where PS is used instead of "PatchSet".
This is not needed. Because if you leave a comment (and do it right, as in, click "Review" on the main gerrit change page under the correct "Patch set" heading) gerrit prefixes this to your comments automatically.
The only reason to need such a prefix is if you're putting it somewhere else, such as the commit-msg – where such comments don't belong in the first place.
Putting it in the commit message is imho annoying because: * It is not at all helpful when the commit is merged because only the last version is merged. The rest are in-between steps during the review process and discussion and details of that process belong in the review thread, not in the commit message. * And, as the author, it is kinda hard because you don't know the patch version number until it is submitted, and someone else can submit a version while you're working.
Having said that, do make sure that your commit message is still accurate and explains the full change (not just the first version of the patch, nor just the last amendment to the patch).
-- Krinkle