Thread (3 messages) 3 messages, 3 authors, 2016-06-15

Re: [PATCH] commit: Add -f, --fixes <commit> option to add Fixes: line

From: Johan Herland <hidden>
Date: 2016-06-15 22:59:08

On Mon, Oct 28, 2013 at 11:10 PM, Thomas Rast [off-list ref] wrote:
Johan Herland [off-list ref] writes:
quoted
But I still don't see exactly what this option should do (inside "git
commit") that would end up being useful across most/all projects, and
not just something that could more easily be implemented in the
*commit-msg hooks for relevant projects.
[Ok, admittedly I don't really know what to quote from your message,
since I'm mostly responding to the overall concept.]

I like the idea of putting all that in hooks, but I have two
observations:

* Signed-off-by: is already such a case (and was probably also added for
  the kernel?) that _could_ have been dealt with using {prepare-,}commit-msg,
  but has its own support in various git tools.
Yes, and I don't like using the precedent of "Signed-off-by" as an
argument to push support for more (IMHO project-specific) footers into
core Git. Hence, I'd rather see the "Signed-off-by" reimplemented as a
hook (obviously, the -s option for "git commit" would have to remain
for backward-compatibility).
* In your list
quoted
  Fixes:
  Reported-by:
  Suggested-by:
  Improved-by:
  Acked-by:
  Reviewed-by:
  Tested-by:
  Signed-off-by:
  and I might add

    Cherry-picked-from:
    Reverts:

  if one were to phrase that as a footer/pseudoheader, observe that
  there are only two kinds of these: footers that contain identities,
  and footers that contain references to commits.
I'm not so sure we can make those assumptions. One might conceivably
imagine a "Fixes:" footer that refers to a bug ID, and not a commit.
Also, projects might want to apply different rules on what may appear
in which footer. E.g. one could e.g. want to enforce that the ident
listed in "Reviewed-by:" or "Signed-off-by:" must always appear in a
project-specific REVIEWERS.txt or AUTHORS.txt file. Since we don't
really know what projects might want, we shouldn't make too many
assumptions on how these footers will be used... That said, I am not
(or at least no longer) opposed to generic support in core Git for
processing these footers, as long as that support is flexible/generic
in nature, and equally available to be reused from within hooks as
from within core Git.
So why not support these use-cases?  We could have something like
footer.foo.* configuration, e.g.

[footer "fixes"]
        type = commit
        suggest = true
[footer "acked-by"]
        type = identity

where 'suggest' (please suggest a better name) means that git-commit
will put a blank one in the commit message template for you to fill in.
'commit' and 'identity' can have some elementary expansion and
validation tied to them.  Some easy extensiblity (hooks?) might not
hurt, but then as you point out, the existing hooks already cover that.

Perhaps we could also have, for Gerrit (cf. [1]):

[footer "change-id"]
        type = uuid

though admittedly I haven't investigated if it's okay to just put a
random string there, or it needs to have a specific value.

[1]  http://thread.gmane.org/gmane.comp.version-control.git/236429
How the config ends up looking is not actually that interesting to me
(not at this stage, at least). My objection is to adding support for
specific footers with specific interpretations tailored specifically
for one (or a few) projects. Such things only open the door to more
bloat. Instead, we already have the hooks for implementing such
project-specific rules and conventions. This is the core of my
argument. Since then, the discussion has moved towards generic and
flexible support for commonly-used footers, and I don't really have a
problem with that, as long as it is easily reusable (and extensible)
by a project's own hooks.

...Johan

-- 
Johan Herland, [off-list ref]
www.herland.net
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help