This is a ridiculous conversation and I can't believe it now spans +20 messages.
___Don't___ -1 a patchset for a typo. The result of that is far more catastrophic. We put off volunteers and people end up wasting valuable time doing rebases due to the time taken to find another code review. Code review is hard enough as it is.
Make a ___suggestion___ about fixing the typo so the person who ends up merging it can do it themselves if necessary.
+1 or -1 depending on the __code quality__ alone.
Can we now return our attention to engaging in more interesting and useful conversations to the future of MediaWiki such as "[Wikitech-l] Disambiguation features: Do they belong in core or in an extension?"
On Tue, Jan 15, 2013 at 10:09 PM, Tim Starling tstarling@wikimedia.org wrote:
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
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l