Pushing patches

Jan Provaznik jprovazn at redhat.com
Thu May 17 09:05:40 UTC 2012


On 05/16/2012 05:28 PM, Matt Wagner wrote:
> On Wed, May 16, 2012 at 10:03:17AM -0400, Jason Guiditta wrote:
>> Hey, list, had a thought on pushing patches.  I have noticed some
>> people (inconsistently) pushing patches others send after they review
>> them, and adding a 'signed-off by' to the patch.  I kind of like this,
>> as it adds a simple way to see who reviewed a patch.
>
> This is easily accomplished with adding "-s" to your git-am run.
>
>> This can make it
>> easy to ask that person 'hey, I am hitting this problem after that
>> patch, did you see this when you tested it?'.  Right now, it is a
>> matter of combingthe list to find the ack, and if you delete your
>> local copies of pushed patches (as I do), that means going to the
>> archive, just t see who reviewed a patch so you can ask them a
>> question.  I would personally much rather just do 'git log' and see
>> who it was, so I can get hold of them as needed.  Thoughts, concerns,
>> vehement opposition?  General apathy?
>
> Actually, I think I do have some opposition. :-\ Adding a signed-off-by
> line is nice, but it implies that the reviewer is the one that pushes.
> This is all well and good, except I think that we're going to have to
> start juggling multiple branches soon, as we backport things to
> maintenance branches.
>
> Compounding matters, I don't think many people really enjoy doing patch
> reviews -- it's just something we begrudgingly do to help get patches
> approved. (I can't say I speak for everyone, but I do know that given
> the choice of writing code or reviewing someone's patch, I'm not the
> only one that would choose to keep coding.) If reviewing someone's patch
> suddenly saddles you with the responsibility of figuring out what
> branches it needs to go onto, pushing it to all of them, and resolving
> any conflicts that might arise, it's going to become considerably more
> unappealing than it already is.
>
> Just MHO, of course.
>
> -- Matt


OT:
Honestly, I like both parts (writing, reviewing) because:
- reading others code is sometimes inspiring
- you can look at a review as a challenge if you are able to break the 
code (find weaknesses)
- it's easiest place where you can effect quality of code

Though it's quite difficult/annoying to keep both in balance because:
- it's quite difficult to keep overview if some patch needs a review or 
not, there are many hands on this project and big traffic on the list 
(where are those times when it was possible to read through all patches 
on the list)
- most patches land on the list few days before sprint ends, and doing 
only reviews couple of days is not very attractive

even more OT:
What is really demotivating for me is if I send a patch which then waits 
week(s) on the list and I have to beg for reviewing it, I hope this gets 
better when we start using a review tool.

Jan



More information about the aeolus-devel mailing list