Re: [PATCH v8 4/8] notes: allow use of the "rewrite" terminology for merge strategies

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Mon, Aug 17, 2015 at 5:54 AM, Johan Herland <johan@xxxxxxxxxxx> wrote:
> On Mon, Aug 17, 2015 at 10:46 AM, Jacob Keller <jacob.e.keller@xxxxxxxxx> wrote:
>> From: Jacob Keller <jacob.keller@xxxxxxxxx>
>>
>> notes-merge.c already re-uses the same functions for the automatic merge
>> strategies used by the rewrite functionality. Teach the -s/--strategy
>> option how to interpret the equivalent rewrite terminology for
>> consistency.
>
> I'm somewhat negative to this patch. IMHO, adding the rewrite modes as
> merge strategy synonyms adds no benefit - only potential confusion -
> to the existing merge strategies. Words that have a sensible meaning
> in the context of rewrite, do not necessarily have the same sensible
> meaning in the context of merge (and vice versa). I'd rather have the
> rewrite code map ignore/overwrite/concatenate to ours/theirs/union,
> without teaching the notes-merge code about these words. Or maybe even
> drop this patch (and the next?) entirely, and let the future author
> (who implements notes rewrite in terms of notes merge) decide how to
> deal with this? By committing to these synonyms now, you might
> actually be making things harder for the future author: once the
> synonyms are part of the user-visible and documented interface, they
> cannot easily be removed/changed again.
>
> ...Johan
>

I am ok dropping these, I really only added them after Junio brought
it up. I think that documenting union/concatenate is fine, but I think
that ours/theirs is very confusing. However, I think you're right that
a future author can deal with this when working on rewrite -> merge. I
don't think we need to do this now.

I'll drop these patches, but I will leave the
parse_notes_merge_strategy in it's location here.

We could document concatenate as a synonym of union but I don't think
that is a big deal.

Regards,
Jake
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]