OK, I see you have a lot of initial committers so the RTC can work fine, I suppose at least. IMHO you can go like this:
We should not put a global timeout, when someone is picking up the patch to be reviewed whether from committers or non-committers, the reviewer SHOULD do the following: 1- The JIRA task related to that patch is RESOLVED and then go to step (2). 2- The committer or non-committer sends an e-mail to sqoop-dev@ asking for review and the subject of the e-mail MUST be tagged with [REVIEW] prefix. 2- Whoever picks the patch first, replies back to the e-mail with the estimated time he/she has, which is a factor you both dropped :D, to review and then commit the patch. 2.1- If OK, then the patch is committed, JIRA is CLOSED, then go to step (3) 2.2- If not OK, then re-open the JIRA task and write your comments and then go to step (3) 3- Send a reply to the [REVIEW] thread 3.1- If patch was OK, send e-mail stating that. 3.2- If patch was not OK, send an e-mail with some detail about where to find the comments, which should be commented on the JIRA task, and hence the cycle is started from the beginning. *NOTE*: We can have a FishEye setup from Atlassian for better code review and managing review requests as well. If that OK I can start the request for Sqoop repository. Thoughts :) ? On Fri, Aug 12, 2011 at 9:03 PM, Arvind Prabhakar <[email protected]> wrote: > On Fri, Aug 12, 2011 at 11:47 AM, Ahmed Radwan <[email protected]> wrote: >> Thanks Arvind, >> >> I prefer RTC with timeout. If we decide on timeout, what is the suitable >> timeout period and how can we manage it for different patches (some patches >> may require more time than others for review)? Is the timeout measured from >> review submission or from last activity on the review? Any ideas? > > Good point Ahmed. I don't think there is any one scheme that will > address all such concerns. Given that someone may view a change as > trivial but it may be very complex for someone else, I think that the > idea of a set timeout will not be fair in all cases. > > Another option to consider is to be like Hive project, where a > committer's patch must be +1'd by another committer who actually > commits it. For patches coming from non-committers, any committer who > reviews it can commit it. > > Thanks, > Arvind > >> >> Best Regards >> Ahmed >> >> On Fri, Aug 12, 2011 at 10:59 AM, Arvind Prabhakar <[email protected]>wrote: >> >>> Hi All, >>> >>> We are starting to see some traction in JIRA and patch activity. I >>> believe now is a good time for us to put a formal policy in place that >>> guides the overall review and commit process. Different projects have >>> adopted different ways of addressing this, but at a high level there >>> are two - Review-Then-Commit (RTC) style, and Commit-Then-Review >>> (CTR). >>> >>> Lets discuss this to bring out various point of views and then do a >>> formal vote on the candidate policy that is acceptable to the >>> majority. >>> >>> My thoughts: I prefer RTC with timeout provisions. Specifically, I >>> feel that every change must get reviewed and if the reviewers do not >>> respond within a certain time, the change can be committed. >>> >>> Please share your thoughts, comments and concerns on this. >>> >>> Thanks, >>> Arvind >>> >> > -- Thanks - Mohammad Nour Author of (WebSphere Application Server Community Edition 2.0 User Guide) http://www.redbooks.ibm.com/abstracts/sg247585.html - LinkedIn: http://www.linkedin.com/in/mnour - Blog: http://tadabborat.blogspot.com ---- "Life is like riding a bicycle. To keep your balance you must keep moving" - Albert Einstein "Writing clean code is what you must do in order to call yourself a professional. There is no reasonable excuse for doing anything less than your best." - Clean Code: A Handbook of Agile Software Craftsmanship "Stay hungry, stay foolish." - Steve Jobs
