On Wednesday, January 1, 2014 8:42:51 AM UTC-10, Jeroen Demeyer wrote: > > On 2014-01-01 19:38, Volker Braun wrote: > > You can only *safely* review a ticket without its dependencies if every > > non-fast-forward change to one of the dependencies resets it back to > > "needs review". > With that philosophy, every time *any ticket* on Trac changes, we should > re-review every other ticket to check for problems. >
Checking for problems is the easy part, this is not what I'm talking about. By *safe*, I mean in a way such that the reviewer, at the time of the review, is actually seeing all the changes that he is reviewing. Whatever proposal you come up with work in the following scenario where there are three commits A -> B -> C and * ticket 1: Commit: B * ticket 2 depends on 1, Commit: C Now you look *only* at commit C and set ticket 2 to positive review. Then somebody else changes ticket one to "Commit: A". Finally, ticket 1 is reviewed. The result is that B is never reviewed but merged into Sage without either reviewer noticing. The solution (I'd call it "Volker's solution" except that its really what everybody else does, so I can't take credit for it) is to tell every reviewer that he is responsible for every commit that is not already reviewed. Its an extremely simple rule. And if its too much work to also review the commits from the dependencies then just start at the dependency. Easy as pie. -- You received this message because you are subscribed to the Google Groups "sage-devel" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at http://groups.google.com/group/sage-devel. For more options, visit https://groups.google.com/groups/opt_out.
