Am 28.06.10 22:45, schrieb Tero Koskinen: >>> 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.
The docs now say > -...@item mtn disapprove @var{id} > +...@item mtn disapprove [...@var{parent}] @var{child} while the command says > Syntax specific to 'mtn disapprove': > > disapprove REVISION [REVISION] Could you please adapt one of the two (the latter might be the better choice) and / or alternativly mention in the command's description which of the two arguments is the parent and which is the child? > +check(get("goodfile", "testfile")) > +commit() > [...] > +check(get("badfile", "testfile")) A small tip: Instead of pulling predefined files with dummy contents from the test directory, it might be easier to just use addfile("goodfile", "dummy content") which creates a new file "goodfile" with the contents "dummy content" and mtn add's it. >> Very cool, code looks good and clean. Some minor obvervations: >> [...] >> 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. Maybe I'm just a little picky, but my original "miuse case" was: $ mtn disapprove h:BRANCH1 h:BRANCH2 which - after resolving the heads for both branches - errors out with mtn: misuse: revision REV1 is not a child of REV2, cannot invert whereas I imagined a message like mtn: misuse: the revision REV1 and REV2 do not share common history could be more helpful. But again, maybe I'm just picky here. It should only be a string change though... Thomas. -- GPG-Key 0x160D1092 | tommyd3...@jabber.ccc.de | http://thomaskeller.biz Please note that according to the EU law on data retention, information on every electronic information exchange might be retained for a period of six months or longer: http://www.vorratsdatenspeicherung.de/?lang=en
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Monotone-devel mailing list Monotone-devel@nongnu.org http://lists.nongnu.org/mailman/listinfo/monotone-devel