Re: git interpret-trailers with multiple keys

On Wed, Apr 6, 2016 at 3:30 PM, Michael S. Tsirkin [off-list ref] wrote:

On Wed, Apr 06, 2016 at 10:42:42AM -0700, Junio C Hamano wrote:

quoted

"Michael S. Tsirkin" [off-list ref] writes:

quoted

On Wed, Apr 06, 2016 at 06:58:30PM +0200, Matthieu Moy wrote:

quoted

"Michael S. Tsirkin" [off-list ref] writes:

quoted

I have this in .git/config

[trailer "r"]
        key = Reviewed-by
        command = "echo \"Michael S. Tsirkin <mst@redhat.com\""
[trailer "s"]
        key = Signed-off-by
        command = "echo \"Michael S. Tsirkin <mst@redhat.com\""

whenever I run git interpret-trailers -t r I see these lines added:

Reviewed-by: Michael S. Tsirkin <mst@redhat.com
Signed-off-by: Michael S. Tsirkin <mst@redhat.com
Reviewed-by: Michael S. Tsirkin <mst@redhat.com

Why is Reviewed-by repeated?  Bug or feature?

The first two lines are added unconditionally:

$ echo | git interpret-trailers

Reviewed-by: Michael S. Tsirkin <mst@redhat.com
Signed-off-by: Michael S. Tsirkin <mst@redhat.com

The last line is added because you've asked for it with --trailer r.

Yes, and because the default is to add the trailer at the end.

quoted

quoted

quoted

I don't think it's currently possible to get the behavior you seem to
expect, ie. to define trailer tokens fully (key and value) in your
config file but use them only on request.

Yes, because you could define for example a function like this:

reviewed() {
    git interpret-trailers --trailer 'Reviewed-by: Michael S. Tsirkin
[off-list ref]' --in-place "$@"
}

So it is kind of easy already to make things requestable.

If people really want some configured trailers to be used only on
request, it is possible to add a config option for that.

quoted

quoted

quoted

(BTW, I think you wanted a closing > at the end)

Is this worth fixing? It doesn't look like a behaviour anyone
would want...

CC'ing Christian who's done the "trailers" thing.

Personally, I do not think adding any configured trailers without
being asked is a sensible behaviour, but it is likely that people
already depend on it, as we seem to see "How do I configure to
always add this and that trailer?" from time to time.  I do not
think it is unreasonable to disable the "automatically add
everything that is configured" when the command line arguments ask
for some specific trailer, but I haven't thought deeply about it.

An additional (uninformed) observation is that the 'echo' looks like
an ugly workaround for the lack of "always use this string as the
value" configuration.

Or at least a default.

quoted

Perhaps next to trailer.<token>.command, we
would need trailer.<token>.value?

Yeah, that is possible too.
It could be bit redundant if we already have a config option to say if
the trailer has to be requested.