Hi, On Wed, 21 Apr 2010 01:20:15 +0200 Thomas Keller wrote:
> > Hi Tero! > > > I have been working on two quickie tasks, two-argument disapprove > > > > At the moment the server carries three branches: > > net.venge.monotone.tkoskine.disapprove-multirev I updated nvm.tkoskine.disapprove-multirev branch at stronglytyped.org server. The branch now contains code, some tests, and a small documentation change. I don't have write access to monotone.ca server, so I cannot push my branch there, but pulling stronglytyped.org shouldn't be a problem. > Very cool, code looks good and clean. Some minor obvervations: > 2) nvm.tkoskine.disapprove-multirev: > > I know the term "changeset" has been used here before, but as > far as I know this is the only place in the complete source tree > and I'd love to see some unification towards "parents", i.e. > "revision %s has %d parents, cannot invert". I changed the code to use "parents" in the messages. > The code which checks if the two given revisions have no merges > and share a common ancestor at all could be improved a bit. If > I trigger it with two distinct revisions from two trees, it > bails out early if it finds the first merge revision from the > start revision's tree, however a much more useful error could > be that both revisions don't share a common ancestor and the > easiest way to determine that is to run erase_ancestors() on > them. If the result is 2, they're two different trees, if its > one, one of them is an ancestor of the other and 0 should fire > an invariant, I guess. Then run walk_revisions and only check > for merge revisions which you explicitely want to exclude. I am not totally sure did I got this right. I use erase_ancestors() to determine early if there are no common ancestors. However, I still do pretty much checking in the walk_revisions function also. > Finally I'd give r and r2 more sounding names, to make it really > clear what the start and what the end revision in the range of > revisions is. r and r2 are now renamed to "parent" and "child". Parent is assumed to be ancestor of child. > Thanks for your work, > Thomas. -- Tero Koskinen <tero.koski...@iki.fi> _______________________________________________ Monotone-devel mailing list Monotone-devel@nongnu.org http://lists.nongnu.org/mailman/listinfo/monotone-devel
