[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Monotone-devel] WIP: multirevision disapprove and clean workspace f
From: |
Tero Koskinen |
Subject: |
Re: [Monotone-devel] WIP: multirevision disapprove and clean workspace features |
Date: |
Mon, 28 Jun 2010 23:45:12 +0300 |
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 <address@hidden>
- Re: [Monotone-devel] WIP: multirevision disapprove and clean workspace features,
Tero Koskinen <=