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(a)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(a)lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l