2008/8/18 Steve Huston <[EMAIL PROTECTED]>: >> -----Original Message----- >> From: Ted Ross [mailto:[EMAIL PROTECTED] >> >> Robert Greig wrote: >> > OK, FWIW here's what I would do. No review board, just: >> > >> > 1) work on issue and commit changes >> > 2) mail or IM someone asking if he would mind reviewing it >> > 3) assign issue to person doing review and move on until issue is >> > either reopened by reviewer or resolved (or whatever state >> comes next) >> > 4) run jira report regularly of issues in "pending review" >> state with >> > no activity for 5 days. For those issues go back to step (2) with >> > alternative reviewer if necessary. >> > >> > I think it would be useful to get some of the other >> committers' views >> > on what would work for them. >> > >> > RG >> +1 to Robert's suggestion >> >> I think commit-then-review will be more efficient given the >> capabilities of our tools. > > I agree with this as well, modulo two points. These may be covered > already, but I haven't seen evidence of it... > > 1. If this process gets too far between commit and review, there's > increased chance there will be changes add in after the commit that > may confuse the reviewer. > > 2. I haven't seen any sort of log or diary of changes. Something that > would, for example, list the files changed and why. Is this normally > put in the svn commit comments? Explanatory entries in such a place > could help the reviewer get his/her bearings and help understand what > the committer was trying to do. Or is this info expected to be in > jira? > > Also, is there someplace relatively easy to check to see if a > particular commit broke something? > > Thanks, > -Steve
Here are my thoughts on this issue. We need to have something lightweight that actually gets done. We've been at Apache nearly two years and we still don't update JIRAs or provide test details (or even the actual test) for our changes. I'd like to think it was the lack of a documented process but I don't know if that would help. Both the suggestions here have merits but both also ask us to do more than we currently do and for that reason I'm not sure how successful either will be. We can all agree that review and testing are important but unless we can show commitment to those goals then I fear that we are going to continually face large code review session with growing action log. We have previously discussed having component leads and I think that is where we should start here. A few people that are committed to the Qpid project and to the quality of the code that it produces. I would say that we should have a group (2 or 3 people) who are responsible for reviewing incoming changes to a particular component. This should prevent any one person being a blocking in the process. The group should be formed from those that want to dedicate their time to our goals of review and testing. These review groups should document the 'quality bar' that they are checking for so that existing commiters and external patch providers can understand the exit gate of the review process. I still have concerns that changes such as the addition of tests or documentation will be hard to chase up. We need to first of all agree what we are trying to get out of this process change. Are we looking for more control over the committed code? Do we want to ensure we have more thoroughly reviewed code and improved web documentation? Once we can agree on the goals I think it will be easier to look at the path to get there. Regards -- Martin Ritchie
