On 16/01/13 01:57, Daniel Kinzler wrote:
On 15.01.2013 15:06, Tyler Romeo wrote:
I agree with Antoine. Commit messages are part of the permanent history of this project. From now until MediaWiki doesn't exist anymore, anybody can come and look at the change history and the commit messages that go with them. Now you might ask what the possibility is of somebody ever coming across a single commit message that has a typo in it, but when you're using git-blame, git-bisect, or other similar tools, it's very possible.
And then they see a typo. So what? If you look through a mailing list archive or Wikipedia edit comments, you will also see typos.
I'm much more concerned about scaring away new contributors with such nitpicking.
I am also concerned about demotivating people.
Giving a change -1 means that you are asking the developer to take orders from you, under threat of having their work ignored forever. A -1 status can cause a change to be ignored by other reviewers, regardless of its merit.
If the developer can't lower their sense of pride sufficiently to allow them to engage with nitpickers, then the change might be ignored by all concerned for many months.
However, if you give minor negative feedback with +0, the change stays bold in your "review requests" list, as if you haven't reviewed it at all. I've tried giving -1 with a comment to the effect of "please merge this immediately regardless of my nitpicking above", but IIRC the comment was ignored.
I think people who give -1 should be aware of the potential roadblock they are creating. And I would like to see a feature in Gerrit to unbold +0 reviews.
Under Subversion, my policy as a reviewer was to never ask the committer to fix a typo in a comment, since committing the fix myself was easier than telling them what to fix, and doing so avoided offence. Under Gerrit, it's more difficult to submit amendments, and I hate multi-author patchsets anyway, so negative feedback seems more attractive.
Maybe the answer is better scripting for amendments and dependent commits.
-- Tim Starling