On 02/20/2014 01:22 PM, 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
I named it to be consistent to other fields: Reported by Owned by So maybe the name could be "Patch Review by"? Martin _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel