Re: [Monotone-devel] Approval revisited...

[Richard Levitte - VMS Whacker]

In message <address@hidden> on Sun, 12 Feb 2006 09:41:14 +1100, Daniel Carosone 
<address@hidden> said:

dan> On Sat, Feb 11, 2006 at 03:37:01AM -0800, Justin Patrin wrote:
dan> > > Personally, I think the functionality of 'disapprove' should
dan> > > move to 'revert' ('revert -r REV [RESTRICTION]; commit'), and
dan> > > 'approve' could just go away, or stay on until we have a real
dan> > > story, or whatever.
dan> > 
dan> > The name 'revert' of course makes sense for this, but this will
dan> > also clash with the 'revert' used to revert a change in a
dan> > working copy....
dan> 
dan> This is exactly the same command.

No.

dan> Notice the -r REV, which means to revert the [RESTRICTION] files
dan> to as they were in REV (as opposed to the BASE rev as default),
dan> and a commit after.  Thus a revert is just like an update that
dan> doesn't change MT/revision, so diffs and commits are still
dan> relative to the original BASE.
dan> 
dan> This works fine if your BASE is the revision where the 'bad'
dan> change was first made, or at least some near descendent before
dan> other changes have been made in the relevant files too.  If
dan> intervening changes have been made, the current functionality of
dan> 'disapprove' is to apply a reverse patch against the current
dan> head.

Incorrect.  disapprove does what you describe it should do, as
follows, except for the merge that you have to do separately:

dan> That's handy and convenient, but I actually think the more
dan> appropriate way to achieve this is to actually go back up the
dan> graph and checkout the original damaged BASE rev, revert and
dan> commit as above, and then merge the new head with the anti-change
dan> with the current head.
dan> 
dan> The resulting graph just makes more sense: it connects the bad
dan> change with the repair, and shows you the alternate path in
dan> between that's affected by the bad code.

I entirely agree with that last remark, and since that's what
disapprove does (well, except for the merge), why should we change it,
and even more, why should we mix it up with another command that does
something different altogether?

dan> So, "yes please".

My vote goes to "no!  pretty please, no!"

Cheers,
Richard

-----
Please consider sponsoring my work on free software.
See http://www.free.lp.se/sponsoring.html for details.

-- 
Richard Levitte                         address@hidden
                                        http://richard.levitte.org/

"When I became a man I put away childish things, including
 the fear of childishness and the desire to be very grown up."
                                                -- C.S. Lewis