John, if the ultimate goal of RTC is to ensure quality then I think the restart guidelines sound reasonable since they guarantee that the exact code being committed has been inspected multiple times and by independent sources. But if the goal of RTC is really just to ensure that "the left hand knows what the right hand is doing" then we may benefit from relaxing the definition of trivial to mean those changes which don't affect the core flow of the reviewed code. e.g. changes which improve readability, address minor oversights, or address edge cases would fall into this category.
Actually, now that I think about it I wonder if allowing these types of changes to go in without restarting the review might actually improve quality because the contributor would otherwise be hesitant to make them. For example, if 3 PMC members review the code and two provide a +1 but a third suggests some "trivial" changes then the contributor is less likely to make them if it means restarting the review. Paul On 7/4/06, John Sisson <[EMAIL PROTECTED]> wrote:
Jason Dillon wrote: > So far 2+ days, several patches... one PMC +1, one non-PMC +1 (with > caveat to ping JVZ)... now crazy problems with diff/patch.. which I'm > not exactly sure how that affects the current votes... or does adding > a new version of the patch negate anything else voted upon. IMO, a vote for a patch would be need to be restarted if the changes between the previous patch and the newer version of the patch are not trivial. Trival meaning: * documentation changes * non-controversial non-semantic style changes such as fixing indentation and adding comments. Trivial changes do not include code changes or changes that affect the operation of the build. To make it easier for reviewers when a new version of a patch is generated, it would be worthwhile adding a comment on what has changed since the previous patch. Do the above patch vote negation guidelines sound reasonable to everyone? Thanks, John
