Re: [PATCH v4 4/4] notes: teach git-notes about notes.<ref>.merge option

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

 



Johan Herland <johan@xxxxxxxxxxx> writes:

> I know that we don't yet have a "proper" place to put remote notes refs,
> but the <ref> in notes.<ref>.merge _must_ be a "local" notes ref (you even
> use the <localref> notation in the documentation below). Thus, I believe
> we can presume that the local notes ref must live under refs/notes/*,
> and hence drop the "refs/notes/" prefix from the config key (just like
> branch.<name>.* can presume that <name> lives under refs/heads/*).

I am OK going in that direction, as long as we promise that "notes
merge" will forever refuse to work on --notes=$ref where $ref does
not begin with refs/notes/.

> Except that this patch in its current form will occupy the .merge config
> key...
>
> Can you rename to notes.<name>.mergestrategy instead?

This is an excellent suggestion.

> Or even better, take inspiration from branch.<name>.mergeoptions,

Please don't.

That is one of the design mistakes that was copied from another
design mistake (remotes.*.tagopt).  I'd want to see us not to repeat
these design mistakes.

These configuration variables were made to take free-form text value
that is split according to shell rules, primarily because it was
expedient to implement.  Read its value into a $variable and put it
at the end of the command line to let the shell split it.  "tagopt"
was done a bit more carefully in that it made to react only with a
fixed string "--no-tags", so it was hard to abuse, but "mergeoptions"
allowed you to override something that you wouldn't want to (e.g. it
even allowed you to feed '--message=foo').

Once you start from such a broken design, it would be hard to later
make it saner, even if you wanted to.  You have to retroactively
forbid something that "worked" (with some definition of "working"),
or you have to split, parse and then reject something that does not
make sense yourself, reimplementing dequote/split rule used in the
shell---which is especially problematic when you no longer write in
shell scripts.

So a single string value that names one of the supported strategy
stored in notes.<name>.mergestrategy is an excellent choice.  An
arbitrary string in notes.<name>.mergeoptions is to be avoided.

Thanks.

--
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]