On 4 January 2014 13:24, Nathann Cohen <[email protected]> wrote: > Hellooooooooooo !! > >> Btw, I'm still not convinced by Volker's explanations, > > Well, by the look of that thread I don't think anybody but Volker was > actually convinced :-P > >> and the policy I >> was suggesting at the beginning of that thread still makes more sense to >> me. In summary, here's what I was suggesting: >> >> 1. The default diff/commits view of a ticket T linked from its trac page >> should be something like >> >> git log --graph commit ^trac/develop ^dep1 ^dep2 ... > > Yes. A big "YES" > > >> where commit is the contents of T's Commit field and dep1, dep2... >> are the contents of the Commit fields of the tickets D1, D2... listed >> in T's Dependencies field. >> >> Setting T to positive_review means that the set of commits described >> by the above "git log" command has been reviewed. > > Indeed. Actually, now that I understand GIt better, it really feels like we > should not be reviewing branches but rewieving *COMMITS*. This way we would > know which commits we can use. Reviewing branches does not seem to make any > sense in a git workflow. I mean, it's the source of all problems we have > right now.
This is quite logical, since a branch is a pointer to a commit and not some fixed thing, so if I say "I have reviewed branch u/x/y/z then it tells you absolutely nothing, while if I say that I am happy with commit a1b2c3d4... then it does refer to a specific snapshot of the code. With the hg model we were reviewing patches, or sequences of patches considered as a single unit, and "positive review" meant something like "I approve of the combined effect of applying these patches". One difficulty with reviewing individual commits is that many of them are snapshots along the way to a finished piece of work. For example, I am about to make a commit of some work I was in the middle of yesterday, so that I can come back to it later, while in the meantime I want to be able to change branches so that I can review some other tickets. It would not make sense for that half-way commit to ever be reviewed in isolation. Of course, the half-way commit only exists on my own computer so I could rewrite local history later, but why should I bother? Does it make sense for us to mark a specific commit as "ready for review" rather than a branch? Working back through that commit's history one might pass by what I called half-way commits above, which could be ignored, until reaching either (1) a commit which has already been merged -- in which case the changes to be reviewed are the ones between that one and the current one; or (2) a commit which is also tagged "ready for review"; or (3) a commit which has a positive review, which could be treated as in case (0). Ok, I know that the number of cases here is more than the ones described; but I have already found that when reviewing a ticket it has been necessary to say in the comment on trac exactly which commit I was looking at, and also when marking a ticket as ready for review I intend to always say which commit it is which I would like reviewed. It would not be hard for trac to have fields showing that information? You might think it redundant, but yesterday someone with initials JD made a new commit to a branch after I had started reviewing a ticket tagged "ready for review"! John > >> If anyone can explain why "Volker's model" is safer than that, I'm very >> interested. > > And if somebody agrees that this is not a good model, I would be interested > too. > > Nathann > > -- > 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. -- 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.
