Jeff King <peff@xxxxxxxx> writes: > On Thu, May 01, 2008 at 10:34:44AM +0200, Santi Béjar wrote: > >> > > 1) the committer ident is different from the parent >> > > 2) the committer ident is set automatically >> > >> > Honestly, I think just "2)" is probably fine (where automatically >> > presumably means "from GECOS"). I see what you are trying to accomplish >> > with "1)", but it's so workflow specific as to be useless. >> >> I don't see "1) as workflow specific, it is there to minimize the >> number of time it is shown, ie somebody might prefer the automatic >> ident, so as long as it does not change it is not shown. > > Yes, but whether it actually succeeds in minimizing is dependent on the > workflow of the user. I.e., it works great if I tend to build on small, > personal repositories (or topic branches), or if I tend to receive other > people's work by patches that I commit myself. But if I am pulling from > somebody else's repo (as many of us do from git.git), or if I use a > shared repo with the other committers, then I am very likely to be > building on a parent that was committed by somebody else (and > consequently will see the committer mentioned way more often than was > intended). The goal of the patch as I understand it is to prevent mistakes of committing under a wrong committer ident, isn't it? Why does "minimizing" come into the picture? Once you have a good algorithm to see when to trigger the warning that the user might be using an unintended committer identity, I do not think you should refrain from issuing the warning when you see the offending committer ident and whose commit you are building on top of should not affect it. Otherwise, the user will get the warning once (or not even get the warning because the commit was made with a "commit -s -m 'typofix'" one liner), and keep using the wrong ident without noticing before building up a long chain of commits. If the identification algorithm is bogus, that would result in too many false hits to be annoying, and in that case, (1) I would not apply such a bogus algorithm until it gets into reasonable shape, (2) we would improve it after it gets applied and if still makes noise on a hopefully rarer false positive cases, and/or (3) the warning will be accompanied with a suggestion to explicitly use user.name and/or user.email in the configuration file to allow the user to squelch it. I think the other patch about showing the author when you are committing other's changes is a good move, by the way. -- 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
