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

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

Reply via email to