| Message ID | fa781cb7c5be82f4293c5aa8697bf5464a9ad5ce.1697667818.git.clement@lassieur.org |
|---|---|
| State | New |
| Headers | show |
| Series | [bug#66618] etc: gitconfig: Remove the default email address to avoid mistakes. | expand |
Hi Clément, Clément Lassieur <clement@lassieur.org> writes: > * etc/git/gitconfig: Remove the default email address to avoid mistakes that > could lead to send private patches to guix-patches@gnu.org. > --- > > This just happened to me twice in a row, so I imagine it could happen to some > other people as well. > > etc/git/gitconfig | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/etc/git/gitconfig b/etc/git/gitconfig > index 907ad0180454..3e94fe801e90 100644 > --- a/etc/git/gitconfig > +++ b/etc/git/gitconfig > @@ -13,6 +13,5 @@ > rebase = true > > [sendemail] > - to = guix-patches@gnu.org > headerCmd = etc/teams.scm cc-members-header-cmd > thread = no Having guix-patches@gnu.org as the default endpoint of 'git send-email' seems reasonable when working on the Guix checkout. From my perspective, it is a useful default to me at least, so without more information as to how that happened, I'm inclined to close this as notabug :-).
Hi Clément, Clément Lassieur <clement@lassieur.org> writes: > * etc/git/gitconfig: Remove the default email address to avoid mistakes that > could lead to send private patches to guix-patches@gnu.org. > --- > > This just happened to me twice in a row, so I imagine it could happen to some > other people as well. That's part of learning 'git send-email', I'd think :-). On a positive note, now you get to learn how to use Debbugs for the same price ;-).
> > This just happened to me twice in a row, so I imagine it could happen to some > > other people as well. > > That's part of learning 'git send-email', I'd think :-). On a positive > note, now you get to learn how to use Debbugs for the same price ;-). People don't expect their git/config to be modified in such substantial ways. It's already difficult for new comers to use git send-email well, please don't make it harder. (And I'm not even talking about the crash.) There this git send-email feature: > The header of the email is configurable via command-line options. If not specified on the command line, the user will be prompted with a ReadLine enabled interface to provide the necessary information. that has been removed with your patch. I call this a bug. I prefer the "wontfix" tag if you really don't want to amend your patch. :-) Thanks, Clément
> Having guix-patches@gnu.org as the default endpoint of 'git send-email' > seems reasonable when working on the Guix checkout. From my > perspective, it is a useful default to me at least, so without more > information as to how that happened, I'm inclined to close this as notabug > :-). Here is the information about what happened: I used --reply-to instead of --to. This should have happen to tell me about my mistake: > To whom should the emails be sent (if anyone)? (plus readline prompt) But nothing happened and the mail went to guix-patches@gnu.org.
Hi, Clément Lassieur <clement@lassieur.org> writes: >> > This just happened to me twice in a row, so I imagine it could happen to some >> > other people as well. >> >> That's part of learning 'git send-email', I'd think :-). On a positive >> note, now you get to learn how to use Debbugs for the same price ;-). > > People don't expect their git/config to be modified in such > substantial ways. It's already difficult for new comers to use git > send-email well, please don't make it harder. (And I'm not even > talking about the crash.) > > There this git send-email feature: > >> The header of the email is configurable via command-line >> options. If not specified on the command line, the user will be >> prompted with a ReadLine enabled interface to provide the necessary >> information. > > that has been removed with your patch. I call this a bug. One could argue it's also a misleading documentation. It should say if a 'to' address is not specified *or* configured. What would you have typed interactively if prompted; guix-patches@gnu.org? :-) I think you might grow to like this default after this initial mishap. > I prefer the "wontfix" tag if you really don't want to amend your patch. :-) OK! I'll leave this opened for a bit longer to allow for others to tip in, after which I'll do so.
Maxim Cournoyer <maxim.cournoyer@gmail.com> writes: >>> The header of the email is configurable via command-line >>> options. If not specified on the command line, the user will be >>> prompted with a ReadLine enabled interface to provide the necessary >>> information. >> >> that has been removed with your patch. I call this a bug. > > One could argue it's also a misleading documentation. It should say if > a 'to' address is not specified *or* configured. What would you have > typed interactively if prompted; guix-patches@gnu.org? :-) I think you > might grow to like this default after this initial mishap. I would have typed the correct bug number, the one I passed to --reply-to instead of --to. What problem does it solve anyway? People that are too lazy to type the address? (Most people automate this) What problem does it create? The tool doesn't behave like people expect. That's some unwanted magic. >> I prefer the "wontfix" tag if you really don't want to amend your patch. :-) > > OK! I'll leave this opened for a bit longer to allow for others to tip > in, after which I'll do so.
Hi Maxim, On Thu, 19 Oct 2023 at 21:54, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote: > One could argue it's also a misleading documentation. It should say if > a 'to' address is not specified *or* configured. Well, personally I prefer: « Explicit is better than implicit. » and « Errors should never pass silently. Unless explicitly silenced. ». Here, the default [sendemail] to = guix-patches@gnu.org makes something implicit – which is not necessary bad – but it hides potential errors – which is not good. IIUC, Clément use-case is the following: git send-email --reply-to=12345@debbugs.gnu.org v2-0001-blabla.patch which is an incorrect use of git-send-email. Because of the default value ’sendemail.to’ from .git/config, the patch v2-0001-blabla.patch is sent to guix-patches@gnu.org. At best, Clément spots that and merges the new created issue with #12345. At worse, it falls into the crack. In all cases, it is burden. Without the default value, the mistake is directly pointed out; git-send-email asks the value for To: since no one is provided. With the default value, it does not teach how to correctly use git-send-email and worse it hides the incorrect usage. > What would you have > typed interactively if prompted; guix-patches@gnu.org? :-) Yes, but it teaches me that I must provide the To: field. Well, I do not see which difficulty the default value solves. However, I see which difficulty it introduces. :-) > OK! I'll leave this opened for a bit longer to allow for others to tip > in, after which I'll do so. Somehow, I think Clément’s suggestion seems more POLA. But I do not mind much since I always use ’--to=’. ;-) Cheers, simon
Hi Simon, Simon Tournier <zimon.toutoune@gmail.com> writes: > Hi Maxim, > > On Thu, 19 Oct 2023 at 21:54, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote: > >> One could argue it's also a misleading documentation. It should say if >> a 'to' address is not specified *or* configured. > > Well, personally I prefer: « Explicit is better than implicit. » and > « Errors should never pass silently. Unless explicitly silenced. ». > > Here, the default > > [sendemail] > to = guix-patches@gnu.org > > makes something implicit – which is not necessary bad – but it hides > potential errors – which is not good. But that's true for any automation of git. If we follow that logic we can remove the other git configurations because they may be surprising, and the send-email hook that notifies people, because that can again be surprising. Obviously, I'd rather not go that slope. The small cost in surprise is offset by removing manual work for myself and I'd expect/hope other committers. Does that make sense?
Maxim Cournoyer <maxim.cournoyer@gmail.com> writes: > Hi Simon, > > Simon Tournier <zimon.toutoune@gmail.com> writes: > >> Hi Maxim, >> >> On Thu, 19 Oct 2023 at 21:54, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote: >> >>> One could argue it's also a misleading documentation. It should say if >>> a 'to' address is not specified *or* configured. >> >> Well, personally I prefer: « Explicit is better than implicit. » and >> « Errors should never pass silently. Unless explicitly silenced. ». >> >> Here, the default >> >> [sendemail] >> to = guix-patches@gnu.org >> >> makes something implicit – which is not necessary bad – but it hides >> potential errors – which is not good. > > But that's true for any automation of git. If we follow that logic we > can remove the other git configurations because they may be surprising, > and the send-email hook that notifies people, because that can again be > surprising. Some automation is fine (although maybe not necessary) because it's about things we want to enforce (like signed commits). > Obviously, I'd rather not go that slope. The small cost in surprise is > offset by removing manual work for myself and I'd expect/hope other > committers. Does that make sense? I don't think there is a need for `--to` automation in Guix repo because it's something the user can automate in their own ~/.gitconfig (with `includeIf`[0]). [0]: https://git-scm.com/docs/git-config#_conditional_includes Clément
Hi, Clément Lassieur <clement@lassieur.org> writes: > Maxim Cournoyer <maxim.cournoyer@gmail.com> writes: > >> Hi Simon, >> >> Simon Tournier <zimon.toutoune@gmail.com> writes: >> >>> Hi Maxim, >>> >>> On Thu, 19 Oct 2023 at 21:54, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote: >>> >>>> One could argue it's also a misleading documentation. It should say if >>>> a 'to' address is not specified *or* configured. >>> >>> Well, personally I prefer: « Explicit is better than implicit. » and >>> « Errors should never pass silently. Unless explicitly silenced. ». >>> >>> Here, the default >>> >>> [sendemail] >>> to = guix-patches@gnu.org >>> >>> makes something implicit – which is not necessary bad – but it hides >>> potential errors – which is not good. >> >> But that's true for any automation of git. If we follow that logic we >> can remove the other git configurations because they may be surprising, >> and the send-email hook that notifies people, because that can again be >> surprising. > > Some automation is fine (although maybe not necessary) because it's > about things we want to enforce (like signed commits). > >> Obviously, I'd rather not go that slope. The small cost in surprise is >> offset by removing manual work for myself and I'd expect/hope other >> committers. Does that make sense? > > I don't think there is a need for `--to` automation in Guix repo because > it's something the user can automate in their own ~/.gitconfig (with > `includeIf`[0]). I often have single patches, or with series I need to send the cover letter first; just writing 'git send-email -1' is shorter than 'git send-email --to=guix-patches@gnu.org -1' :-). I also often use patman (info "(u-boot) Patman patch manager") and there's no way to specify a --to from the command line (you use Series-to: NNNNN@debbugs.gnu.org as a git trailer after having received it for a cover letter), which would make things slightly more annoying... > [0]: https://git-scm.com/docs/git-config#_conditional_includes ... by having to configure this in a way that matches all my Guix worktrees spread around :-). I guess what I'm saying is that it provides value for me, and I'd expect for others, although I can see how it could generate surprise when you initially get your 'get send-email' foo wrong. Perhaps we should more prominently mention that 'mumi send-email' should be used for first timers, as it's less likely to get things wrong using it?
Hi Maxim, On Fri, 20 Oct 2023 at 11:44, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote: >> Here, the default >> >> [sendemail] >> to = guix-patches@gnu.org >> >> makes something implicit – which is not necessary bad – but it hides >> potential errors – which is not good. > > But that's true for any automation of git. I disagree. The aim of automation is to hide burden without being error-prone. > If we follow that logic we > can remove the other git configurations because they may be surprising, The point is about being able to catch and adjust accordingly. Except this ’sendemail.to’ value, all the other values may be surprising but if one is surprised, then one is aware of the non-default Git configuration and thus one is able to adjust accordingly. The issue with ’sendemail.to’ is that there is no surprise popping; for being surprised, I need to be aware and here it does it in my back. > and the send-email hook that notifies people, because that can again be > surprising. I disagree. For example, I think ’sendemail.headerCmd’ is a good default. Because it hides the burden and correct my potential mistake. Well, somehow there is “good” surprise and “bad” surprise. For me, ’sendemail.to’ leads to “bad” surprise and ’sendemail.headerCmd’ leads to “good” surprise. For me, ’sendemail.headerCmd’ helps because it behaves in a way that most users will expect the system to behave. That’s POLA. However, ’sendemail.to’ does not appear to me POLA because it hides my potential mistake. > The small cost in surprise is > offset by removing manual work for myself and I'd expect/hope other > committers. Well, from my point of view, it is possible to use other means for removing some manual work. For instance, it is possible to have an alias in etc/gitconfig, as “git send-patches”. Or it is possible to use ’sendemail.aliasesFile’ for having a short key to pass. Or else. You said: OK! I'll leave this opened for a bit longer to allow for others to tip in, after which I'll do so. and now you have two opinions by two Guix committers :-) – not git-send-email experts though but having some habits for using it. ;-) That’s said, I do not have a strong opinion either. I can live with the default value since I always explicitly uses ’--to=’. :-) And if this default value suits your productivity, it is totally fine with me. Cheers, simon
Hi Maxim, On Sun, 22 Oct 2023 at 20:30, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote: > I guess what I'm saying is that it provides value for me, and I'd expect > for others, although I can see how it could generate surprise when you > initially get your 'get send-email' foo wrong. Do you have your configuration public somewhere? Or part of it. Because if I do not see the value it provides, maybe I am missing some features that you are using and eases the workflow. Cheers, simon
Hi Simon, Simon Tournier <zimon.toutoune@gmail.com> writes: > Hi Maxim, > > On Sun, 22 Oct 2023 at 20:30, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote: > >> I guess what I'm saying is that it provides value for me, and I'd expect >> for others, although I can see how it could generate surprise when you >> initially get your 'get send-email' foo wrong. > > Do you have your configuration public somewhere? Or part of it. > > Because if I do not see the value it provides, maybe I am missing some > features that you are using and eases the workflow. It's already included in the Guix repo as .patman. If you have a single patch to send, you can do just: $ patman Review the patch content, C-x 3 when done (if your editor is Emacs), and done. I had documented the flow in https://issues.guix.gnu.org/issue/58813#11, before settling on adding new features to git itself so that we could stick to 'git send-email', as there was some (understandable) reluctance to learning yet another tool [0]. It's also well documented as (info ("u-boot") Patman patch m anager). [0] https://issues.guix.gnu.org/issue/58813#11
Hi Simon, Simon Tournier <zimon.toutoune@gmail.com> writes: > Hi Maxim, > > On Fri, 20 Oct 2023 at 11:44, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote: > >>> Here, the default >>> >>> [sendemail] >>> to = guix-patches@gnu.org >>> >>> makes something implicit – which is not necessary bad – but it hides >>> potential errors – which is not good. >> >> But that's true for any automation of git. > > I disagree. The aim of automation is to hide burden without being > error-prone. > > >> If we follow that logic we >> can remove the other git configurations because they may be surprising, > > The point is about being able to catch and adjust accordingly. Except > this ’sendemail.to’ value, all the other values may be surprising but if > one is surprised, then one is aware of the non-default Git configuration > and thus one is able to adjust accordingly. The same hold for send-email.to. You may get surprised once, and then you know it. The 'git send-email' command will prompt before sending the email with something like: --8<---------------cut here---------------start------------->8--- $ git send-email -1 ;;; note: source file /home/maxim/src/guix/etc/teams.scm ;;; newer than compiled /home/maxim/.cache/guile/ccache/3.0-LE-8-4.6/home/maxim/src/guix/etc/teams.scm.go /tmp/YXDTZOc_v8/0001-wip-lfs.patch ;;; note: source file /home/maxim/src/guix/etc/teams.scm ;;; newer than compiled /home/maxim/.cache/guile/ccache/3.0-LE-8-4.6/home/maxim/src/guix/etc/teams.scm.go (mbox) Adding cc: Maxim Cournoyer <maxim.cournoyer@gmail.com> from line 'From: Maxim Cournoyer <maxim.cournoyer@gmail.com>' From: [...] To: guix-patches@gnu.org Cc: [...] Subject: [PATCH] wip lfs Date: Mon, 23 Oct 2023 10:11:24 -0400 Message-ID: <88ec0c8ccd84fe5e746c66259b8a41d25022f2f0.1698070284.git.maxim.cournoyer@gmail.com> X-Mailer: git-send-email 2.41.0 MIME-Version: 1.0 X-Debbugs-Cc: [...] Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit The Cc list above has been expanded by additional addresses found in the patch commit message. By default send-email prompts before sending whenever this occurs. This behavior is controlled by the sendemail.confirm configuration setting. For additional information, run 'git send-email --help'. To retain the current behavior, but squelch this message, run 'git config --global sendemail.confirm auto'. Send this email? ([y]es|[n]o|[e]dit|[q]uit|[a]ll): --8<---------------cut here---------------end--------------->8--- The 'To: ' field is visible in this summary before confirming whether the email should be sent. [...] > I disagree. For example, I think ’sendemail.headerCmd’ is a good > default. Because it hides the burden and correct my potential mistake. > > Well, somehow there is “good” surprise and “bad” surprise. For me, > ’sendemail.to’ leads to “bad” surprise and ’sendemail.headerCmd’ leads > to “good” surprise. > > For me, ’sendemail.headerCmd’ helps because it behaves in a way that > most users will expect the system to behave. That’s POLA. However, > ’sendemail.to’ does not appear to me POLA because it hides my potential > mistake. That's subjective unfortunately; at least two committers were originally surprised by the then new 'sendemail.headerCmd' behavior. >> The small cost in surprise is >> offset by removing manual work for myself and I'd expect/hope other >> committers. > > Well, from my point of view, it is possible to use other means for > removing some manual work. For instance, it is possible to have an > alias in etc/gitconfig, as “git send-patches”. Or it is possible to use > ’sendemail.aliasesFile’ for having a short key to pass. Or else. > > You said: > > OK! I'll leave this opened for a bit longer to allow for others to tip > in, after which I'll do so. > > and now you have two opinions by two Guix committers :-) – not > git-send-email experts though but having some habits for using it. ;-) > > That’s said, I do not have a strong opinion either. I can live with the > default value since I always explicitly uses ’--to=’. :-) And if this > default value suits your productivity, it is totally fine with me. It does! I don't see how forcing everyone to explicitly type --to=guix-patches@debbugs.gnu.org for sending simple patches or cover letters helps our flow. Mistakes will happen, but that's human, and we can merge or close the issue on Debbugs when they occur, without loosing any sleep :-)
Maxim Cournoyer <maxim.cournoyer@gmail.com> writes: >> I don't think there is a need for `--to` automation in Guix repo because >> it's something the user can automate in their own ~/.gitconfig (with >> `includeIf`[0]). > > I often have single patches, or with series I need to send the cover > letter first; just writing 'git send-email -1' is shorter > than 'git send-email --to=guix-patches@gnu.org -1' :-). Well, you can use a bash alias, a git send-email wrapper, your own .gitconfig as I said... There are a lot of solutions. > I also often use patman (info "(u-boot) Patman patch manager") and > there's no way to specify a --to from the command line (you use > Series-to: NNNNN@debbugs.gnu.org as a git trailer after having received > it for a cover letter), which would make things slightly more > annoying... Then you can also write "Series-to: guix-patches@gnu.org" as trailer for the first patch and then change. Or you can use git send-email --to for the first patch, and patman for the remaining ones. >> [0]: https://git-scm.com/docs/git-config#_conditional_includes > > ... by having to configure this in a way that matches all my Guix > worktrees spread around :-). If all your Guix worktrees are in the same directory, it's straightforward. Otherwise, it's just one line per git repository. Something that's surely not a problem for you. > I guess what I'm saying is that it provides value for me, and I'd expect > for others, although I can see how it could generate surprise when you > initially get your 'get send-email' foo wrong. I'm sure it provides value for you but I don't think it does for others: all I see is that it provides confusion. We want a software, and tools, that are simple to use and to understand. Please don't add too much things that are hard to maintain, hard to understand, and with unexpected defaults. Git send-email is used by a lot of projects apart from Guix. And newcomers often come from those projects, they don't expect weird defaults and weird customizations. > Perhaps we should more prominently mention that 'mumi send-email' should > be used for first timers, as it's less likely to get things wrong using > it? I think git send-email is easy to use and to understand (if people don't modify its default behavior). Thanks, Clément
Hi Clément, On Wed, 25 Oct 2023 at 22:11, Clément Lassieur <clement@lassieur.org> wrote: > I'm sure it provides value for you but I don't think it does for others: > all I see is that it provides confusion. > > We want a software, and tools, that are simple to use and to understand. > Please don't add too much things that are hard to maintain, hard to > understand, and with unexpected defaults. Although I tend to agree with the weak value of ’sendemail.to’ – until now, I have not seen what it brings, I think you are overemphasizing the issue here. :-) At worst, a misuse leads to open many new bug numbers. Future will say if this adds more burden than help. On average, it should simplify the workflow and free some cognitive overhead; that’s the idea. Somehow, the misguided use-case you initially reported is a specific case. The bet is: these specific cases are by a very large order of magnitude less than the day-to-day cases, and thus we can deal with specific cases when they happen and it reduces the overhead for most. Cheers, simon
On Thu, Oct 26 2023, Simon Tournier wrote: > Hi Clément, > > On Wed, 25 Oct 2023 at 22:11, Clément Lassieur <clement@lassieur.org> wrote: > >> I'm sure it provides value for you but I don't think it does for others: >> all I see is that it provides confusion. >> >> We want a software, and tools, that are simple to use and to understand. >> Please don't add too much things that are hard to maintain, hard to >> understand, and with unexpected defaults. > > Although I tend to agree with the weak value of ’sendemail.to’ – until > now, I have not seen what it brings, I think you are overemphasizing the > issue here. :-) Alright, sorry for overemphasizing then. I'm closing this issue now with tag "wontfix" because it's bad for Guix that we are all wasting time on this. Thanks, Clément
diff --git a/etc/git/gitconfig b/etc/git/gitconfig index 907ad0180454..3e94fe801e90 100644 --- a/etc/git/gitconfig +++ b/etc/git/gitconfig @@ -13,6 +13,5 @@ rebase = true [sendemail] - to = guix-patches@gnu.org headerCmd = etc/teams.scm cc-members-header-cmd thread = no