[Freeipa-devel] Reviewer in Trac

[Petr Spacek]

On 20.2.2014 14:31, Jan Cholasta wrote:
> On 20.2.2014 13:14, 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.
>
> Is it possible to add new ticket states in Trac? I'm thinking extending it so
> that instead of "new -> assigned -> closed:fixed" we have "new ->
> assigned:work in progress -> assigned:patch available -> assigned:patch under
> review -> closed:fixed", or something like that.

This sounds like
http://trac.edgewall.org/wiki/TracWorkflow
see sections
"Example: Add simple optional generic review state"
and
"Example: Adding optional Testing with Workflow".

BTW the text mentions "How to combine the tracopt.ticket.commit_updater with 
the testing workflow": If I understand correctly, it allows you to close 
tickers in post-commit hook and things like that.

Petr^2 Spacek

>> 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]
>> http://www.freeipa.org/page/Contribute/Code#Tracking_patches_.28Experimental.29