Re: [Svnmerge] [PATCH] Add ability to mark change sets as merged

[Giovanni Bajo <ra...@develer.com>]

Daniel Rall <dl...@collab.net> wrote:

> This is effectively a way to manipulate the merge memory stored in the
> "svnmerge-integrated" property.  It's the flip-side of the 'block'
> concept.

Hi Dan, thanks for the patch. It looks ok.

I don't specifically like the name of the option nor the documentation. I
thought of "--manual": what people think of it? I'm not really satisfied with
it either. "--record-only" is longer but possibly better? I don't think it's
too long as I don't expect it to be used so commonly to justify the need of a
shorter name (and there's the single-dash letter version anyway). [BTW, I don't
care about the single-dash letter, -M: that's always a non-informative
non-mnemonic version of the option, so really any letter works).

So a proposal:

"""
--record-only: do not perform an actual merge of the changes, yet record that a
merge happened.
"""

As for the main description at the top:

"""
When manually merging changes across branches, --record-only can be used to
instruct svnmerge that a manual merge of a certain revision already happened,
so that it can record it and not offer that revision for merge anymore.
"""

I'd also add a note to the svnmerge block help page:

"""
Do not use this option to hide revisions that were manually merged into the
branch. Instead, use "svnmerge merge --record-only". The immediate effect is
the same (the revision is not available for merge anymore), but the behaviour
will be more correct with respect to merge across multiple branches.
"""

Giovanni Bajo


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [Svnmerge] [PATCH] Add ability to mark change sets as merged

[Giovanni Bajo <ra...@develer.com>]

David James <dj...@collab.net> wrote:


>> """
>> [...]
>> The immediate effect is the same (the revision is not available for
>> merge anymore), but the behaviour will be more correct with respect
>> to merge across multiple branches.
>> """
>
> Really? Why would --record-only allow svnmerge.py to behave more
> correctly?

Lie to svnmerge.py and it will take its revenge :)

You manual-merge r100 from A to B, and block it in B (instead of
using --record-only). Then you merge all the changes from B to C: this
effectively brings in the manual merge of r100, but C doesn't know about it.
You then merge everything from A to C, and you get conflicts because r100 is
merged again.

I'm not sure svnmerge.py is *currently* smart enough that, if you
use --record-only, it understands that it doesn't need to merge r100 from A to
C. Surely, it is a feature that it is feasable. Even if it doesn't right now,
my point that you shouldn't lie to svnmerge still stands.

Anyway, I'm satisfied with what was committed right now as part of the help of
'block'. It is clear enough.

Giovanni Bajo


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [Svnmerge] [PATCH] Add ability to mark change sets as merged

[Daniel Rall <dl...@collab.net>]

Giovanni, David, thanks for the excellent feedback.

On Sat, 25 Mar 2006, David James wrote:

> On 3/25/06, Giovanni Bajo <ra...@develer.com> wrote:
> > Daniel Rall <dl...@collab.net> wrote:
> >
> > > This is effectively a way to manipulate the merge memory stored in the
> > > "svnmerge-integrated" property.  It's the flip-side of the 'block'
> > > concept.
> >
> > Hi Dan, thanks for the patch. It looks ok.
> >
> > I don't specifically like the name of the option nor the documentation. I
> > thought of "--manual": what people think of it? I'm not really satisfied with
> > it either. "--record-only" is longer but possibly better? I don't think it's
> > too long as I don't expect it to be used so commonly to justify the need of a
> > shorter name (and there's the single-dash letter version anyway). [BTW, I don't
> > care about the single-dash letter, -M: that's always a non-informative
> > non-mnemonic version of the option, so really any letter works).
> >
> > So a proposal:
> >
> > """
> > --record-only: do not perform an actual merge of the changes, yet record that a
> > merge happened.
> > """
> 
> I definitely like these new names. "--manual" describes the use case:
> I did a manual merge; please update the merge metadata for me.
> "--record-only" describes what svnmerge.py actually does -- it records
> that we already did a manual merge. Both names are good, with the
> first being shorter, and the second being slightly more clear. Like
> you, I tend to favour clarity over brevity.

I actually considered both these names, and am happy with
--record-only too.

It's nice when the short option also reflects the long option.  I
considering switching the short option from -M to -O (-R reminds me
too much of Subverion's own "recursive"), but after conversation with
David, left it as -M on the grounds that it's a better mnemonic for
"manual" and "merge".

> > As for the main description at the top:
> >
> > """
> > When manually merging changes across branches, --record-only can be used to
> > instruct svnmerge that a manual merge of a certain revision already happened,
> > so that it can record it and not offer that revision for merge anymore.
> > """
> 
> Very clear. Great! Thanks for writing this documentation as it will be
> very helpful for users.

Looks great, thanks Giovanni!

> Should we also refer back to the "block" subcommand, which allows you
> to ask svnmerge *not* to merge a particular set of revisions? In some
> cases, if you didn't exactly "merge" the revision but the
> functionality is available on the branch with similar code, it might
> be tough to choose between the two.

I mentioned it.

> We may want to mention somewhere that svnmerge.py does not record
> "indirect" metadata (i.e. it does not record that, if you merge a
> change from branchA to branchB, you might indirectly be merging
> changes from trunk). For now, you can correct this metadata using
> merge --record-only. In future, it might be possible for us to be a
> bit better about merging the indirect metadata, but I don't think
> we'll ever get it perfect, so this feature will always be handy.
> 
> > I'd also add a note to the svnmerge block help page:
> >
> > """
> > Do not use this option to hide revisions that were manually merged into the
> > branch. Instead, use "svnmerge merge --record-only".
> 
> Great! Good idea to refer back to "--record-only".

I used this text.

> > The immediate effect is the same (the revision is not available for merge anymore),
> > but the behaviour will be more correct with respect to merge across multiple branches.
> 
> Really? Why would --record-only allow svnmerge.py to behave more
> correctly? I find this sentence a bit confusing. I think we should
> simply say that "--record-only" records that a merge happened, whereas
> "block" records that a merge should not happen.

I used something closer to David's suggestion, and committed the
changes to Subversion trunk in r19051.  If you'd like any additional
tweaks, let me know.
-- 

Daniel Rall

Re: [Svnmerge] [PATCH] Add ability to mark change sets as merged

[David James <dj...@collab.net>]

On 3/25/06, Giovanni Bajo <ra...@develer.com> wrote:
> Daniel Rall <dl...@collab.net> wrote:
>
> > This is effectively a way to manipulate the merge memory stored in the
> > "svnmerge-integrated" property.  It's the flip-side of the 'block'
> > concept.
>
> Hi Dan, thanks for the patch. It looks ok.
>
> I don't specifically like the name of the option nor the documentation. I
> thought of "--manual": what people think of it? I'm not really satisfied with
> it either. "--record-only" is longer but possibly better? I don't think it's
> too long as I don't expect it to be used so commonly to justify the need of a
> shorter name (and there's the single-dash letter version anyway). [BTW, I don't
> care about the single-dash letter, -M: that's always a non-informative
> non-mnemonic version of the option, so really any letter works).
>
> So a proposal:
>
> """
> --record-only: do not perform an actual merge of the changes, yet record that a
> merge happened.
> """

I definitely like these new names. "--manual" describes the use case:
I did a manual merge; please update the merge metadata for me.
"--record-only" describes what svnmerge.py actually does -- it records
that we already did a manual merge. Both names are good, with the
first being shorter, and the second being slightly more clear. Like
you, I tend to favour clarity over brevity.

> As for the main description at the top:
>
> """
> When manually merging changes across branches, --record-only can be used to
> instruct svnmerge that a manual merge of a certain revision already happened,
> so that it can record it and not offer that revision for merge anymore.
> """

Very clear. Great! Thanks for writing this documentation as it will be
very helpful for users.

Should we also refer back to the "block" subcommand, which allows you
to ask svnmerge *not* to merge a particular set of revisions? In some
cases, if you didn't exactly "merge" the revision but the
functionality is available on the branch with similar code, it might
be tough to choose between the two.

We may want to mention somewhere that svnmerge.py does not record
"indirect" metadata (i.e. it does not record that, if you merge a
change from branchA to branchB, you might indirectly be merging
changes from trunk). For now, you can correct this metadata using
merge --record-only. In future, it might be possible for us to be a
bit better about merging the indirect metadata, but I don't think
we'll ever get it perfect, so this feature will always be handy.

> I'd also add a note to the svnmerge block help page:
>
> """
> Do not use this option to hide revisions that were manually merged into the
> branch. Instead, use "svnmerge merge --record-only".

Great! Good idea to refer back to "--record-only".

> The immediate effect is the same (the revision is not available for merge anymore),
> but the behaviour will be more correct with respect to merge across multiple branches.

Really? Why would --record-only allow svnmerge.py to behave more
correctly? I find this sentence a bit confusing. I think we should
simply say that "--record-only" records that a merge happened, whereas
"block" records that a merge should not happen.

Cheers,

David

--
David James -- http://www.cs.toronto.edu/~james

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org