Re: git interpret-trailers with multiple keys

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Apr 06, 2016 at 10:42:42AM -0700, Junio C Hamano wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Wed, Apr 06, 2016 at 06:58:30PM +0200, Matthieu Moy wrote:
> >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> >> 
> >> > 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.
> >> 
> >> 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.
> >> 
> >> (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.

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


-- 
MST