On 01/30/2012 05:39 AM, Tomas Sedovic wrote:
On 01/26/2012 09:13 PM, Jason Guiditta wrote:
> At the suggestion of jclift, I am resending this email as its own
> thread in the hopes that more people will notice it.
>
> I am a bit disconcerted by the number of 'self-acks' I have been
> seeing on list lately. I know there were some complaints about speed
> of reviews a while back, which led (I thought) to the community
> agreeing that if you had a trivial fix/patch that you were confident
> broke nothing, you could go ahead and push. This was not meant, in my
> estimation anyway, to give everyone open license to just push whatever
> they want with no review. We should still be reviewing and testing
> patches properly, and if you have something that has been sitting on
> list waiting for review (like, ahem, my mysql-fixing patch, someone
> please look at it!), it is up to you to make a nuisance of yourself in
> whatever way you want until you get a reviewer to work with you.
>
> This is not directed at anyone specific, there have just been too many
> of late, and I could not sit quietly and watch them go by anymore.
>
> -j
I've been guilty of this lately. It was lazy, I'm sorry.
Jay, I completely agree with you on this. It's okay to self-ack
trivial patches but anything exceeding more than a couple of lines
should still be reviewed properly.
I would argue that it is content not quantity that matters the most here.
A patch containing a ton of whitespace changes or spelling corrections
or a functional fix to some ui component could probably be self-ack'd if
the author is sure it works as intended (the last part is key to the
self-ack process).
A two line patch containing changing core functionality of the app that
others will need to know about should be reviewed.
-Mo