On 02/20/2014 03:52 PM, Jakub Hrozek wrote:
> On Thu, Feb 20, 2014 at 01:22:56PM +0100, Petr Viktorin wrote:
>> On 02/20/2014 01:14 PM, Martin Kosek wrote:
>>> We had a discussion with other developers how better track who is reviewing
>>> which patch. Recently, we introduced the Reviewed-By tag in a commit 
>>> message,
>>> but that is a post-review tag which is not useful for someone who wants to 
>>> know
>>> which patches are already reviewed and which are not reviewed.
>>>
>>> We were testing Patch Work [1] in last months to contain this information, 
>>> but
>>> I personally think that it is suboptimal - it introduces 2 tracking tools 
>>> that
>>> needs to be maintained (Trac and Patch Work) and the Patch Work still 
>>> requires
>>> lot of manual actions when maintaining it's state.
>>>
>>> I think it would be better to hold this information rather in a single 
>>> tracking
>>> tool - Trac. There are 2 options:
>>>
>>> 1) "Patch on review" flag, similar to "Patch posted for review" flag which
>>> would hold 1 bit information if the patch is just lying there or has 
>>> somebody
>>> assigned.
>>>
>>> 2) "Reviewed by" text field which would hold a login of a person who is
>>> reviewing it. It would be filled either by a person starting the review or 
>>> by a
>>> supervisor like me to forcefully assign a reviewer ;-)
>>>
>>> With that information in Trac, we could run using a single tracking tool for
>>> all patches that have a ticket (which is 95% of patches). It would be then
>>> fairly easy to see which patches are sent for review but are reviewer-less.
>>>
>>> It would also have a benefit for Petr's sendpatches.py script which could 
>>> pull
>>> the reviewer from a ticket and one would not have to use the "-r" option to
>>> hard code a reviewer.
>>>
>>> Any objections to using "Reviewed by" field?
>>
>> +1, this is the only thing I used Patchwork for, and keeping
>> Patchwork up to date just for this took a lot of unnecessary
>> mindless clicking.
>>
>> Just a nitpick: name it "Patch Reviewer"
>> - there's more to a ticket than a patch
>> - the review is not done yet when the field is filled out
> 
> The only use-case I use patchwork for right now is a 'dashboard' to see
> which patches need attention. If we could get this dashboard-like view
> from Trac with some custom query, then I'm fine with deprecating
> Patchwork.

+1. I would like to add the reviewer to default report 3 + prepare a new view
"My Active Reviews by Milestone" to see the reviews which one should track.

> 
> However, one feature of patchwork was that each re-submission of a
> patch triggered a new thread so as a reviewer you could easily see there
> is a new instance of the patch that you need to look at. I suspect Trac
> wouldn't give us anything like that?

When I get a review, I like to get the response to inbox - then I always know.
When replies are only being sent to the list, we would have to use the advanced
Trac workflow and cautiously change states between accepted - submitted - 
onreview.

Martin

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to