On (20/02/14 15:09), Martin Kosek wrote:
>On 02/20/2014 02:54 PM, Petr Spacek wrote:
>> On 20.2.2014 14:47, Martin Kosek wrote:
>>> On 02/20/2014 02:31 PM, 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.
>>> It is possible to change the workflow, yes - this is something I was also
>>> considering.
>>> It can be done with this plugin:
>>> https://trac-hacks.org/wiki/AdvancedTicketWorkflowPlugin
>>> Unfortunately, the plugin that's in Fedorahosted Trac does not work 
>>> properly,
>>> it gave me some Internal Errors. I filed a ticket for that:
>>> https://fedorahosted.org/fedora-infrastructure/ticket/4237
>>> When it is fixed, we can try to think about adjusting the workflow. Maybe we
>>> can indeed add new states "submitted" and "onreview" to the workflow. But 
>>> even
>>> then I think we could use the "Patch Review by" field so that we know who is
>>> reviewing, if anybody.
>> Thinking a bit more the e-mail notifications ...
>> We can reassign ticket to reviewer if we have state "onreview". IMHO ticket
>> owner always get e-mails, right?
>> Nice side-effect is that report "{4} Assigned, Active Tickets by Owner" will
>> give you exact list of open tickets (state != new && state != closed) and you
>> will see who is in charge for each ticket right in the report.
>> Ticket owner is not set in stone and everything is still in Trac history (and
>> Git, of course :-).
>Maybe. But that would mean that when you NACK a patch, you would need to move
>the ticket back to previous state to reset the owner. As I know how much you
>like the bureaucracy, are you sure you want this change? :)
>When the ticket is closed I personally like that owner is set to the
>implementer. As that way I quickly see who I need to yell when this ticket
>breaks something.
>IMO ideal state would be to have a workflow:
>"Accept as reviewer" - adds your name to reviewer field, moves to onreview
>"Assign reviewer: ________" - adds some login to reviewer field, moves to 
>We would just need to find out how to do this with some Workflow plugin when it
>is ready.
I could not resist.

This effort with the trac looks like reinventing wheel (gerrit or similar tool)


Freeipa-devel mailing list

Reply via email to