[Monotone-devel] Re: monotone disapprove does not give correct branch cert

[Wim Oudshoorn]

Nathaniel Smith <address@hidden> writes:

> On Tue, Oct 25, 2005 at 07:53:23PM +0200, Wim Oudshoorn wrote:
>> Emile Snyder <address@hidden> writes:
>> 
>> > Yuck.  cert.cc:guess_branch(revision) defaults to using
>> > app.branch_name() if one is set; ie. you are in a working copy.
>> > There are 4 commands using guess_branch to decide how to cert a new
>> > revision:
>> >
>> > approve
>> > disapprove
>> > checkout
>> > commit
>> 
>> >From these only commit and disapprove will create a new revision
>> in the database.   So approve and checkout should not add any branch
>> certificate.
>
> checkout doesn't create any branch certificates; it calls guess_branch
> to figure out what the appropriate default branch for the working copy
> should be.
>
> "approve" is short for "add a branch certificate"; that's all it does.
> So I think it should add a branch certificate :-).

I stand corrected.  I never use approve and somehow I thought
it was a shortcut for 'this revision did pass the tests'.

>> But disapprove you give an explicit revision, so in which directory
>> you are should be irrelevant.  AFAICT there are two more or less
>
>> reasonable options,
>> 
>>   disapprove REV
>> 
>> should get:
>> 
>> * all branch certificates from REV
>> or
>> * no branch certificate at all.
>> 
>> I lean towards the second options for the following reason,
>> it is not clear beforehand what the full collection of branch
>> certificates of REV is.  A sync can add new branch certificates 
>> to an existing revision. 

> Fortunately, I think these goals are consistent here, because at the
> more abstract level, branches are all about intentions -- putting a
> revision in a branch is how we say that that revision is good for that
> branches purpose.  'disapprove', now, is entirely about intentions --
> we don't mean "this change was bad", we always have some purpose in
> mind, "this change is bad for purpose X".  So disapprove revisions
> should have branch certs on them.

Of course.  But what do you do in the following situation:

rev A                rev B                        rev C
branch 1  ------->   branch 3  -- disapprove -->  branch(es) ???
branch 2             branch 4                          


Also.  I have the uncomfortable feeling that the disapprove should
not be on a revision, but should be on an edge instead. 
AFAICT, but I haven't tried it yet, you can't disapprove of a merge,
or a propagate


rev A
branch 1  ----->    rev C        --- disapprove???  
                    branch 1
               / 
rev B         / propagate
branch 2     


But suppose the above works I can immediately tell that it will not work
in the following situation, which btw is quite common in our setup:


rev A        ......................   XXX 
branch main 
         \                            /
           rev B         rev C       /  propagate back to main
           branch fix    branch fix


now if you try the above you don't get a nice new node rev D
with branch certificate, you get just a branch certificate

branch main 

added to rev C.   This in itself is not bad, but very unfortantely
monotone does not even add an edge between rev A and rev C to 
indicate that a propagate has taken place.  

Suppose you want to disapprove the propagate from branch fix
to branch main because the fix was faulty and need some more development.
How to do this cleanly?

I realize I drifted somewhat from the original topic,
but disapprove in my mind is not really flushed out yet
and it can't hurt to think about this.  
Oh and it falls for me in the same category is some other 
nitpicking I have with the refusal of empty commits etc.
When I have a clear head and some time I really really
should write these things down.  

Wim Oudshoorn