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 >onreview > >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) LS _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel