| Message ID | 20190711183727.8058-1-ariadne@dereferenced.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | use mailmap by default in git log | expand |
Ariadne Conill <ariadne@dereferenced.org> writes: > It is not uncommon for people to change their name or e-mail address. > To facilitate this, Git provides support for the `.mailmap` file, > which contains a list of identities and previously used e-mail > addresses that are associated with that identity. > > Unfortunately, while Git's support for the `.mailmap` file is generally > excellent, I recently discovered that `git log` does not treat the > mail map file the same as the other tools, instead requiring an > explicit flag to use the mailmap file. Make "the other tools" a bit more explicit. Making things consistent is good but which way the consistency should go needs more data than the above to decide. Even though I personally think it is an OK longer-term end goal, the execution looks too hasty. The normal way we handle a big behaviour change like this is to do the following in steps, in different releases: - In the first release, introduce an early adoptor option (say log.usemailmap) that can be turned on by the user, but is off by default. IOW, the initial step is "no change in behaviour, unless you ask for it". This step also makes sure that the way to disable it for those who opt into the option from the command line (i.e. the --no-use-mailmap option) works well. - In the second release, when "git log" is run without command line "--[no-]use-mailmap" and "log.usemailmap" is not set by the user, give warning about an upcoming flipping of the default, with an advice message that the user can squelch the warning by setting the option. - In the final release, flip the default and remove the warning. Usually there needs sufficient time between the second step and the third step, so that people will not miss the warning. Thanks.
Junio C Hamano <gitster@pobox.com> writes: > Even though I personally think it is an OK longer-term end goal, the > execution looks too hasty. The normal way we handle a big behaviour > change like this is to do the following in steps, in different > releases: > > - In the first release, introduce an early adoptor option (say > log.usemailmap) that can be turned on by the user, but is off by > default. IOW, the initial step is "no change in behaviour, > unless you ask for it". This step also makes sure that the way > to disable it for those who opt into the option from the command > line (i.e. the --no-use-mailmap option) works well. > > - In the second release, when "git log" is run without command line > "--[no-]use-mailmap" and "log.usemailmap" is not set by the user, > give warning about an upcoming flipping of the default, with an > advice message that the user can squelch the warning by setting > the option. > > - In the final release, flip the default and remove the warning. > > Usually there needs sufficient time between the second step and the > third step, so that people will not miss the warning. IIUC, we are between step 1 and step 2. The configuration already exists and uses the safe (i.e. the same as before) default. Your change combines the step 2 and step 3 into one, which will not work. What we need at this point is the "second release" phase, i.e. additional warnings without yet changing the default behaviour. After it is given to the end users and sufficient time passes, we can flip the default.
Hello, On Thu, Jul 11, 2019 at 2:34 PM Junio C Hamano <gitster@pobox.com> wrote: > > Junio C Hamano <gitster@pobox.com> writes: > > > Even though I personally think it is an OK longer-term end goal, the > > execution looks too hasty. The normal way we handle a big behaviour > > change like this is to do the following in steps, in different > > releases: > > > > - In the first release, introduce an early adoptor option (say > > log.usemailmap) that can be turned on by the user, but is off by > > default. IOW, the initial step is "no change in behaviour, > > unless you ask for it". This step also makes sure that the way > > to disable it for those who opt into the option from the command > > line (i.e. the --no-use-mailmap option) works well. > > > > - In the second release, when "git log" is run without command line > > "--[no-]use-mailmap" and "log.usemailmap" is not set by the user, > > give warning about an upcoming flipping of the default, with an > > advice message that the user can squelch the warning by setting > > the option. > > > > - In the final release, flip the default and remove the warning. > > > > Usually there needs sufficient time between the second step and the > > third step, so that people will not miss the warning. > > IIUC, we are between step 1 and step 2. The configuration already > exists and uses the safe (i.e. the same as before) default. Your > change combines the step 2 and step 3 into one, which will not work. Makes sense. > What we need at this point is the "second release" phase, i.e. > additional warnings without yet changing the default behaviour. > After it is given to the end users and sufficient time passes, we > can flip the default. Do you have a proposed timetable for this? I can add a warning message and we can proceed with the warning message for now and then flip the defaults later. I just need to know what version you would like to do the flip in (3.0?) so that I can write the warning message. Assuming the release you would like to flip the setting in is 3.0, I would propose something like this: Warning: The `git log` command will default to using the mailmap file if present to map contributor names as of Git 3.0. If you want to enable this behaviour now, use `git config --global log.mailmap true` to enable it. If you want to explicitly disable this behaviour in the future, use `git config --global log.mailmap false` to disable it. Your thoughts on this message? Ariadne
Ariadne Conill <ariadne@dereferenced.org> writes: >> What we need at this point is the "second release" phase, i.e. >> additional warnings without yet changing the default behaviour. >> After it is given to the end users and sufficient time passes, we >> can flip the default. > > Do you have a proposed timetable for this? I can add a warning > message and we can proceed with the warning message for now and then > flip the defaults later. I just need to know what version you would > like to do the flip in (3.0?) so that I can write the warning message. I do not think we usually do this without having to say "at this release" in such a warning. A recent example of a behaviour change that was backward incompatible was that we no longer allow $ git log -- '' to mean the same thing as $ git log -- . since Git 2.16. This change was initially planned in Git 2.11 timeframe, and we started warning when "git log" is used with an empty string as one of the pathspec elements on the command line in that release. We kept warning for some releases and then at last at Git 2.16 we flipped the switch. It was started at d426430e ("pathspec: warn on empty strings as pathspec", 2016-06-22) and then flipped at 9e4e8a64 ("pathspec: die on empty strings as pathspec", 2017-06-06). Run "git show" on these commits, with pathspec "pathspec.c", to see exact wording we used. You should be able to find other examples by looking in the Documentation/Relnotes directory and finding backward compatibility notes in there. > Warning: The `git log` command will default to using the mailmap file > if present to map contributor names as of Git 3.0. If you want to > enable this behaviour now, use `git config --global log.mailmap true` > to enable it. If you want to explicitly disable this behaviour in the > future, use `git config --global log.mailmap false` to disable it. Other than (1) the explicit "as of ..." which we do not have to say, and (2) use of "--global", as this is pretty much per-project convention and is better handled by default per-repository basis, nto per-user basis, I think the proposed text tries to convey the right message. But again, it is advisable to study how we phrased these warning messages in past releases for different features and mimic them. Thanks.
Junio C Hamano <gitster@pobox.com> writes: > I do not think we usually do this without having to say "at this > release" in such a warning. Sorry for horrible copy-editing that made the result say 100% opposite of what I meant. I am bad at negations. But I hope the mistake and the message I wanted to convey were obvious enough ;-) We've done this without giving exact timeframe in the message.
Hello, On Fri, Jul 12, 2019 at 9:49 AM Junio C Hamano <gitster@pobox.com> wrote: > > Junio C Hamano <gitster@pobox.com> writes: > > > I do not think we usually do this without having to say "at this > > release" in such a warning. > > Sorry for horrible copy-editing that made the result say 100% > opposite of what I meant. I am bad at negations. > > But I hope the mistake and the message I wanted to convey were > obvious enough ;-) We've done this without giving exact timeframe in > the message. Yes, I understood what you were proposing. I believe I have added an adequate deprecation warning in the patches I just sent. Thanks for your guidance on this! Ariadne
On Thu, Jul 11, 2019 at 01:37:24PM -0500, Ariadne Conill wrote: > It is not uncommon for people to change their name or e-mail address. > To facilitate this, Git provides support for the `.mailmap` file, > which contains a list of identities and previously used e-mail > addresses that are associated with that identity. > > Unfortunately, while Git's support for the `.mailmap` file is generally > excellent, I recently discovered that `git log` does not treat the > mail map file the same as the other tools, instead requiring an > explicit flag to use the mailmap file. > > I believe this is an unfortunate flaw, as the mailmap file should > ideally contain the most current known contact information for a > contributor, allowing anyone to contact the contributor about their > patches in the future. > > This should be the finished version of the patch set, thanks to > everyone who has helped review it! Thank you very much for following up on this. I've been meaning to revisit my RFC from last year on this topic for some time but unfortunately Git work has not been able to be a priority for me for some time. I think that this patch everything that covered 'log' specifically that I'm aware of, 'shortlog' still has some issues that I'd like to address. CB