Patchwork patchbomb: make --git imply --plain

login
register
mail settings
Submitter Henning Schild
Date Nov. 16, 2016, 8:41 p.m.
Message ID <181aaf02c1542387651e.1479328861@localhost>
Download mbox | patch
Permalink /patch/17609/
State Rejected
Headers show

Comments

Henning Schild - Nov. 16, 2016, 8:41 p.m.
# HG changeset patch
# User Henning Schild <henning@hennsch.de>
# Date 1479327376 -3600
#      Wed Nov 16 21:16:16 2016 +0100
# Node ID 181aaf02c1542387651e66c22063bb276271ced2
# Parent  db8637a8a7464aca668e5aaa2fb9610edaf6f286
patchbomb: make --git imply --plain

Not using --plain would generate mails with the hg header in the git commit
message. Since --git already caters for git, might as well make sure users do
not forget --plain.
Augie Fackler - Nov. 16, 2016, 10:41 p.m.
> On Nov 16, 2016, at 15:41, Henning Schild <henning@hennsch.de> wrote:
> 
> # HG changeset patch
> # User Henning Schild <henning@hennsch.de>
> # Date 1479327376 -3600
> #      Wed Nov 16 21:16:16 2016 +0100
> # Node ID 181aaf02c1542387651e66c22063bb276271ced2
> # Parent  db8637a8a7464aca668e5aaa2fb9610edaf6f286
> patchbomb: make --git imply --plain

No thanks. --git here is about using the git-diff format, which is still extremely valuable for hg users.

> 
> Not using --plain would generate mails with the hg header in the git commit
> message. Since --git already caters for git, might as well make sure users do
> not forget --plain.
> 
> diff --git a/hgext/patchbomb.py b/hgext/patchbomb.py
> --- a/hgext/patchbomb.py
> +++ b/hgext/patchbomb.py
> @@ -524,6 +524,10 @@
>     # internal option used by pbranches
>     patches = opts.get('patches')
> 
> +    if (opts.get('git') and not opts.get('plain')):
> +        repo.ui.debug('--git implies --plain, setting --plain')
> +        opts['plain'] = True
> +
>     if not (opts.get('test') or mbox):
>         # really sending
>         mail.validateconfig(ui)
> diff --git a/tests/test-patchbomb.t b/tests/test-patchbomb.t
> --- a/tests/test-patchbomb.t
> +++ b/tests/test-patchbomb.t
> @@ -755,13 +755,6 @@
>   To: foo
>   Cc: bar
> 
> -  # HG changeset patch
> -  # User test
> -  # Date 3 0
> -  #      Thu Jan 01 00:00:03 1970 +0000
> -  # Node ID ff2c9fa2018b15fa74b33363bda9527323e2a99f
> -  # Parent  97d72e5f12c7e84f85064aa72e5a297142c36ed9
> -  c
>   ---
>    c |  1 +
>    1 files changed, 1 insertions(+), 0 deletions(-)
> @@ -953,13 +946,6 @@
>   To: foo
>   Cc: bar
> 
> -  # HG changeset patch
> -  # User test
> -  # Date 1 0
> -  #      Thu Jan 01 00:00:01 1970 +0000
> -  # Node ID 8580ff50825a50c8f716709acdf8de0deddcd6ab
> -  # Parent  0000000000000000000000000000000000000000
> -  a
>   ---
>    a |  1 +
>    1 files changed, 1 insertions(+), 0 deletions(-)
> @@ -989,13 +975,6 @@
>   To: foo
>   Cc: bar
> 
> -  # HG changeset patch
> -  # User test
> -  # Date 2 0
> -  #      Thu Jan 01 00:00:02 1970 +0000
> -  # Node ID 97d72e5f12c7e84f85064aa72e5a297142c36ed9
> -  # Parent  8580ff50825a50c8f716709acdf8de0deddcd6ab
> -  b
>   ---
>    b |  1 +
>    1 files changed, 1 insertions(+), 0 deletions(-)
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Sean Farley - Nov. 16, 2016, 10:47 p.m.
Augie Fackler <raf@durin42.com> writes:

>> On Nov 16, 2016, at 15:41, Henning Schild <henning@hennsch.de> wrote:
>> 
>> # HG changeset patch
>> # User Henning Schild <henning@hennsch.de>
>> # Date 1479327376 -3600
>> #      Wed Nov 16 21:16:16 2016 +0100
>> # Node ID 181aaf02c1542387651e66c22063bb276271ced2
>> # Parent  db8637a8a7464aca668e5aaa2fb9610edaf6f286
>> patchbomb: make --git imply --plain
>
> No thanks. --git here is about using the git-diff format, which is still extremely valuable for hg users.

I'm inclined to agree with Augie here. This is a pretty big format
change that might potentially break older clients importing a patch. The
risk / reward ratio doesn't seem strong enough to me. Plus, yes, --git
everywhere that I can find in Mercurial means git-diff format.

Perhaps this patch (or something similar) belongs in hg-git instead?
Augie Fackler - Nov. 16, 2016, 10:49 p.m.
> On Nov 16, 2016, at 17:47, Sean Farley <sean@farley.io> wrote:
> 
> Augie Fackler <raf@durin42.com> writes:
> 
>>> On Nov 16, 2016, at 15:41, Henning Schild <henning@hennsch.de> wrote:
>>> 
>>> # HG changeset patch
>>> # User Henning Schild <henning@hennsch.de>
>>> # Date 1479327376 -3600
>>> #      Wed Nov 16 21:16:16 2016 +0100
>>> # Node ID 181aaf02c1542387651e66c22063bb276271ced2
>>> # Parent  db8637a8a7464aca668e5aaa2fb9610edaf6f286
>>> patchbomb: make --git imply --plain
>> 
>> No thanks. --git here is about using the git-diff format, which is still extremely valuable for hg users.
> 
> I'm inclined to agree with Augie here. This is a pretty big format
> change that might potentially break older clients importing a patch. The
> risk / reward ratio doesn't seem strong enough to me. Plus, yes, --git
> everywhere that I can find in Mercurial means git-diff format.

It's even explicitly documented as the diff format, not the email format:
augie% hg help email | grep -- --git
 -g --git                use git extended diff format

> Perhaps this patch (or something similar) belongs in hg-git instead?
Henning Schild - Nov. 17, 2016, 7:34 a.m.
Am Wed, 16 Nov 2016 17:41:03 -0500
schrieb Augie Fackler <raf@durin42.com>:

> > On Nov 16, 2016, at 15:41, Henning Schild <henning@hennsch.de>
> > wrote:
> > 
> > # HG changeset patch
> > # User Henning Schild <henning@hennsch.de>
> > # Date 1479327376 -3600
> > #      Wed Nov 16 21:16:16 2016 +0100
> > # Node ID 181aaf02c1542387651e66c22063bb276271ced2
> > # Parent  db8637a8a7464aca668e5aaa2fb9610edaf6f286
> > patchbomb: make --git imply --plain  
> 
> No thanks. --git here is about using the git-diff format, which is
> still extremely valuable for hg users.

Ok. I guess that consequently also means no to the diffstat patch (2 of
2 out of this thread)?

Let me see how to fit that change into hg-git. I did not have a look at
how that hooks into hg, but i can imagine a similar patch would be much
harder there - like parsing the mbox. I need that change working with
hg-git for contributing code back to upstream mailing lists.
I can imagine that similar patches would be required for the reverse
operation (hg mimport), but i did not try that way yet.

How about a new option --git-format that implies --git and --plain and
takes care of the diffstat?

> > 
> > Not using --plain would generate mails with the hg header in the
> > git commit message. Since --git already caters for git, might as
> > well make sure users do not forget --plain.
> > 
> > diff --git a/hgext/patchbomb.py b/hgext/patchbomb.py
> > --- a/hgext/patchbomb.py
> > +++ b/hgext/patchbomb.py
> > @@ -524,6 +524,10 @@
> >     # internal option used by pbranches
> >     patches = opts.get('patches')
> > 
> > +    if (opts.get('git') and not opts.get('plain')):
> > +        repo.ui.debug('--git implies --plain, setting --plain')
> > +        opts['plain'] = True
> > +
> >     if not (opts.get('test') or mbox):
> >         # really sending
> >         mail.validateconfig(ui)
> > diff --git a/tests/test-patchbomb.t b/tests/test-patchbomb.t
> > --- a/tests/test-patchbomb.t
> > +++ b/tests/test-patchbomb.t
> > @@ -755,13 +755,6 @@
> >   To: foo
> >   Cc: bar
> > 
> > -  # HG changeset patch
> > -  # User test
> > -  # Date 3 0
> > -  #      Thu Jan 01 00:00:03 1970 +0000
> > -  # Node ID ff2c9fa2018b15fa74b33363bda9527323e2a99f
> > -  # Parent  97d72e5f12c7e84f85064aa72e5a297142c36ed9
> > -  c
> >   ---
> >    c |  1 +
> >    1 files changed, 1 insertions(+), 0 deletions(-)
> > @@ -953,13 +946,6 @@
> >   To: foo
> >   Cc: bar
> > 
> > -  # HG changeset patch
> > -  # User test
> > -  # Date 1 0
> > -  #      Thu Jan 01 00:00:01 1970 +0000
> > -  # Node ID 8580ff50825a50c8f716709acdf8de0deddcd6ab
> > -  # Parent  0000000000000000000000000000000000000000
> > -  a
> >   ---
> >    a |  1 +
> >    1 files changed, 1 insertions(+), 0 deletions(-)
> > @@ -989,13 +975,6 @@
> >   To: foo
> >   Cc: bar
> > 
> > -  # HG changeset patch
> > -  # User test
> > -  # Date 2 0
> > -  #      Thu Jan 01 00:00:02 1970 +0000
> > -  # Node ID 97d72e5f12c7e84f85064aa72e5a297142c36ed9
> > -  # Parent  8580ff50825a50c8f716709acdf8de0deddcd6ab
> > -  b
> >   ---
> >    b |  1 +
> >    1 files changed, 1 insertions(+), 0 deletions(-)
> > _______________________________________________
> > Mercurial-devel mailing list
> > Mercurial-devel@mercurial-scm.org
> > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel  
> 
>
Sean Farley - Nov. 17, 2016, 9:34 p.m.
Henning Schild <henning@hennsch.de> writes:

> Am Wed, 16 Nov 2016 17:41:03 -0500
> schrieb Augie Fackler <raf@durin42.com>:
>
>> > On Nov 16, 2016, at 15:41, Henning Schild <henning@hennsch.de>
>> > wrote:
>> > 
>> > # HG changeset patch
>> > # User Henning Schild <henning@hennsch.de>
>> > # Date 1479327376 -3600
>> > #      Wed Nov 16 21:16:16 2016 +0100
>> > # Node ID 181aaf02c1542387651e66c22063bb276271ced2
>> > # Parent  db8637a8a7464aca668e5aaa2fb9610edaf6f286
>> > patchbomb: make --git imply --plain  
>> 
>> No thanks. --git here is about using the git-diff format, which is
>> still extremely valuable for hg users.
>
> Ok. I guess that consequently also means no to the diffstat patch (2 of
> 2 out of this thread)?
>
> Let me see how to fit that change into hg-git. I did not have a look at
> how that hooks into hg, but i can imagine a similar patch would be much
> harder there - like parsing the mbox. I need that change working with
> hg-git for contributing code back to upstream mailing lists.
> I can imagine that similar patches would be required for the reverse
> operation (hg mimport), but i did not try that way yet.

Since a git mailing list would be only for git clients, it'd be ok to
have both import / export in the git style for hg-git. Maybe others feel
different than me, though.

> How about a new option --git-format that implies --git and --plain and
> takes care of the diffstat?

We usually shy away from new arguments and instead suggest the
improvement of templates which could be used here (maybe they already
can, I haven't checked).
Augie Fackler - Nov. 17, 2016, 9:39 p.m.
On Thu, Nov 17, 2016 at 4:34 PM, Sean Farley <sean@farley.io> wrote:
> Henning Schild <henning@hennsch.de> writes:
>
>> Am Wed, 16 Nov 2016 17:41:03 -0500
>> schrieb Augie Fackler <raf@durin42.com>:
>>
>>> > On Nov 16, 2016, at 15:41, Henning Schild <henning@hennsch.de>
>>> > wrote:
>>> >
>>> > # HG changeset patch
>>> > # User Henning Schild <henning@hennsch.de>
>>> > # Date 1479327376 -3600
>>> > #      Wed Nov 16 21:16:16 2016 +0100
>>> > # Node ID 181aaf02c1542387651e66c22063bb276271ced2
>>> > # Parent  db8637a8a7464aca668e5aaa2fb9610edaf6f286
>>> > patchbomb: make --git imply --plain
>>>
>>> No thanks. --git here is about using the git-diff format, which is
>>> still extremely valuable for hg users.
>>
>> Ok. I guess that consequently also means no to the diffstat patch (2 of
>> 2 out of this thread)?
>>
>> Let me see how to fit that change into hg-git. I did not have a look at
>> how that hooks into hg, but i can imagine a similar patch would be much
>> harder there - like parsing the mbox. I need that change working with
>> hg-git for contributing code back to upstream mailing lists.
>> I can imagine that similar patches would be required for the reverse
>> operation (hg mimport), but i did not try that way yet.
>
> Since a git mailing list would be only for git clients, it'd be ok to
> have both import / export in the git style for hg-git. Maybe others feel
> different than me, though.

It's probably Not Okay to have hg-git break hg email for users that
have hg-git turned on globally, rather than per-repo?

>
>> How about a new option --git-format that implies --git and --plain and
>> takes care of the diffstat?
>
> We usually shy away from new arguments and instead suggest the
> improvement of templates which could be used here (maybe they already
> can, I haven't checked).

I'm not sure that applies entirely to email tho - the email formats
aren't templated, though I suppose they could be...

Now you've got me thinking.
Sean Farley - Nov. 17, 2016, 10:16 p.m.
Augie Fackler <raf@durin42.com> writes:

> On Thu, Nov 17, 2016 at 4:34 PM, Sean Farley <sean@farley.io> wrote:
>> Henning Schild <henning@hennsch.de> writes:
>>
>>> Am Wed, 16 Nov 2016 17:41:03 -0500
>>> schrieb Augie Fackler <raf@durin42.com>:
>>>
>>>> > On Nov 16, 2016, at 15:41, Henning Schild <henning@hennsch.de>
>>>> > wrote:
>>>> >
>>>> > # HG changeset patch
>>>> > # User Henning Schild <henning@hennsch.de>
>>>> > # Date 1479327376 -3600
>>>> > #      Wed Nov 16 21:16:16 2016 +0100
>>>> > # Node ID 181aaf02c1542387651e66c22063bb276271ced2
>>>> > # Parent  db8637a8a7464aca668e5aaa2fb9610edaf6f286
>>>> > patchbomb: make --git imply --plain
>>>>
>>>> No thanks. --git here is about using the git-diff format, which is
>>>> still extremely valuable for hg users.
>>>
>>> Ok. I guess that consequently also means no to the diffstat patch (2 of
>>> 2 out of this thread)?
>>>
>>> Let me see how to fit that change into hg-git. I did not have a look at
>>> how that hooks into hg, but i can imagine a similar patch would be much
>>> harder there - like parsing the mbox. I need that change working with
>>> hg-git for contributing code back to upstream mailing lists.
>>> I can imagine that similar patches would be required for the reverse
>>> operation (hg mimport), but i did not try that way yet.
>>
>> Since a git mailing list would be only for git clients, it'd be ok to
>> have both import / export in the git style for hg-git. Maybe others feel
>> different than me, though.
>
> It's probably Not Okay to have hg-git break hg email for users that
> have hg-git turned on globally, rather than per-repo?

Whoops, yeah, I only meant for a git repo.

>>> How about a new option --git-format that implies --git and --plain and
>>> takes care of the diffstat?
>>
>> We usually shy away from new arguments and instead suggest the
>> improvement of templates which could be used here (maybe they already
>> can, I haven't checked).
>
> I'm not sure that applies entirely to email tho - the email formats
> aren't templated, though I suppose they could be...
>
> Now you've got me thinking.

:-)
Henning Schild - Nov. 17, 2016, 10:33 p.m.
Am Thu, 17 Nov 2016 16:39:05 -0500
schrieb Augie Fackler <raf@durin42.com>:

> On Thu, Nov 17, 2016 at 4:34 PM, Sean Farley <sean@farley.io> wrote:
> > Henning Schild <henning@hennsch.de> writes:
> >  
> >> Am Wed, 16 Nov 2016 17:41:03 -0500
> >> schrieb Augie Fackler <raf@durin42.com>:
> >>  
> >>> > On Nov 16, 2016, at 15:41, Henning Schild <henning@hennsch.de>
> >>> > wrote:
> >>> >
> >>> > # HG changeset patch
> >>> > # User Henning Schild <henning@hennsch.de>
> >>> > # Date 1479327376 -3600
> >>> > #      Wed Nov 16 21:16:16 2016 +0100
> >>> > # Node ID 181aaf02c1542387651e66c22063bb276271ced2
> >>> > # Parent  db8637a8a7464aca668e5aaa2fb9610edaf6f286
> >>> > patchbomb: make --git imply --plain  
> >>>
> >>> No thanks. --git here is about using the git-diff format, which is
> >>> still extremely valuable for hg users.  
> >>
> >> Ok. I guess that consequently also means no to the diffstat patch
> >> (2 of 2 out of this thread)?
> >>
> >> Let me see how to fit that change into hg-git. I did not have a
> >> look at how that hooks into hg, but i can imagine a similar patch
> >> would be much harder there - like parsing the mbox. I need that
> >> change working with hg-git for contributing code back to upstream
> >> mailing lists. I can imagine that similar patches would be
> >> required for the reverse operation (hg mimport), but i did not try
> >> that way yet.  
> >
> > Since a git mailing list would be only for git clients, it'd be ok
> > to have both import / export in the git style for hg-git. Maybe
> > others feel different than me, though.  
> 
> It's probably Not Okay to have hg-git break hg email for users that
> have hg-git turned on globally, rather than per-repo?
> 
> >  
> >> How about a new option --git-format that implies --git and --plain
> >> and takes care of the diffstat?  
> >
> > We usually shy away from new arguments and instead suggest the
> > improvement of templates which could be used here (maybe they
> > already can, I haven't checked).  
> 
> I'm not sure that applies entirely to email tho - the email formats
> aren't templated, though I suppose they could be...

The whole template-story sounds like a good idea but kind of long-term.
For the last series i have sent the question is whether the new
argument would contradict such a change (if hg-git brought its own
template and the new argument would not be required anymore).

Today you can already send valid mails to git-based projects. Including
the diffstat is just a style thing, if you skip it git-am wont care.
Setting --plain and --git automatically is pure convenience.

That being said, sending emails without diffstat - and maybe other
additional information - might result in git upstream to "dislike" the
mails and rejecting the contributions.

While this is not an hg issue it could mean that hg-git is not a
viable option if you are planning to contribute back to upstream git.

> Now you've got me thinking.
>

Patch

diff --git a/hgext/patchbomb.py b/hgext/patchbomb.py
--- a/hgext/patchbomb.py
+++ b/hgext/patchbomb.py
@@ -524,6 +524,10 @@ 
     # internal option used by pbranches
     patches = opts.get('patches')
 
+    if (opts.get('git') and not opts.get('plain')):
+        repo.ui.debug('--git implies --plain, setting --plain')
+        opts['plain'] = True
+
     if not (opts.get('test') or mbox):
         # really sending
         mail.validateconfig(ui)
diff --git a/tests/test-patchbomb.t b/tests/test-patchbomb.t
--- a/tests/test-patchbomb.t
+++ b/tests/test-patchbomb.t
@@ -755,13 +755,6 @@ 
   To: foo
   Cc: bar
   
-  # HG changeset patch
-  # User test
-  # Date 3 0
-  #      Thu Jan 01 00:00:03 1970 +0000
-  # Node ID ff2c9fa2018b15fa74b33363bda9527323e2a99f
-  # Parent  97d72e5f12c7e84f85064aa72e5a297142c36ed9
-  c
   ---
    c |  1 +
    1 files changed, 1 insertions(+), 0 deletions(-)
@@ -953,13 +946,6 @@ 
   To: foo
   Cc: bar
   
-  # HG changeset patch
-  # User test
-  # Date 1 0
-  #      Thu Jan 01 00:00:01 1970 +0000
-  # Node ID 8580ff50825a50c8f716709acdf8de0deddcd6ab
-  # Parent  0000000000000000000000000000000000000000
-  a
   ---
    a |  1 +
    1 files changed, 1 insertions(+), 0 deletions(-)
@@ -989,13 +975,6 @@ 
   To: foo
   Cc: bar
   
-  # HG changeset patch
-  # User test
-  # Date 2 0
-  #      Thu Jan 01 00:00:02 1970 +0000
-  # Node ID 97d72e5f12c7e84f85064aa72e5a297142c36ed9
-  # Parent  8580ff50825a50c8f716709acdf8de0deddcd6ab
-  b
   ---
    b |  1 +
    1 files changed, 1 insertions(+), 0 deletions(-)