Patchwork [3,of,3] patchbomb: introduce a 'patchwork.confirm' option

login
register
mail settings
Submitter Pierre-Yves David
Date Dec. 3, 2014, 3:12 a.m.
Message ID <20e66de4913566197f00.1417576326@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/6970/
State Accepted
Headers show

Comments

Pierre-Yves David - Dec. 3, 2014, 3:12 a.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1417570521 28800
#      Tue Dec 02 17:35:21 2014 -0800
# Node ID 20e66de4913566197f0066f5835086c844deb401
# Parent  d943db1f1f572bf3c01b8ca6b5e728ebc30649aa
patchbomb: introduce a 'patchwork.confirm' option

When set to true, this option will make patchbomb always ask for confirmation
before sending the email. Confirmation is a powerful way to prevent stupid
mistake when the sending patches.

This should let me get ride of my global alias adding
--confirm to hg email.

I know that some people may get bitten when moving from a machine with confirm
configured  to machine where it is not. But I think it is still worth the risk.
Ryan McElroy - Dec. 3, 2014, 5:06 a.m.
I like the idea of this patchset and it seems pretty straightforward. I would certainly use this to help myself from spamming the list with bad patches!

> -----Original Message-----

> From: Mercurial-devel [mailto:mercurial-devel-bounces@selenic.com] On

> Behalf Of Pierre-Yves David

> Sent: Tuesday, December 2, 2014 7:12 PM

> To: mercurial-devel@selenic.com

> Cc: Pierre-Yves David

> Subject: [PATCH 3 of 3] patchbomb: introduce a 'patchwork.confirm' option

> 

> # HG changeset patch

> # User Pierre-Yves David <pierre-yves.david@fb.com> # Date 1417570521

> 28800

> #      Tue Dec 02 17:35:21 2014 -0800

> # Node ID 20e66de4913566197f0066f5835086c844deb401

> # Parent  d943db1f1f572bf3c01b8ca6b5e728ebc30649aa

> patchbomb: introduce a 'patchwork.confirm' option

> 

> When set to true, this option will make patchbomb always ask for

> confirmation before sending the email. Confirmation is a powerful way to

> prevent stupid mistake when the sending patches.

> 

> This should let me get ride of my global alias adding --confirm to hg email.

> 

> I know that some people may get bitten when moving from a machine with

> confirm configured  to machine where it is not. But I think it is still worth the

> risk.

> 

> diff --git a/hgext/patchbomb.py b/hgext/patchbomb.py

> --- a/hgext/patchbomb.py

> +++ b/hgext/patchbomb.py

> @@ -50,10 +50,13 @@ overwritten by command line flags like -

> 

>    [patchbomb]

>    intro=auto   # include introduction message if more than 1 patch (default)

>    intro=never  # never included an introduction message

>    intro=always # always include an introduction message

> +

> +You can set patchbomb to always ask for confirmation by setting

> +``patchbomb.confirm`` to true.

>  '''

> 

>  import os, errno, socket, tempfile, cStringIO  import email  # On python2.4

> you have to import these by name or they fail to @@ -428,13 +431,14 @@

> def patchbomb(ui, repo, *revs, **opts):

>      Then when all is done, patchbomb messages are displayed. If the

>      PAGER environment variable is set, your pager will be fired up once

>      for each patchbomb message, so you can verify everything is alright.

> 

>      In case email sending fails, you will find a backup of your series

> -    introductory message in ``.hg/last-email.txt``. The inclusion the

> -    introduction can also be control using the ``patchbomb.intro`` option. (see

> -    hg help patchbomb for details)

> +    introductory message in ``.hg/last-email.txt``.

> +

> +    The default behavior of this command can be customized through

> +    configuration. (See :hg:`help patchbomb` for details)

> 

>      Examples::

> 

>        hg email -r 3000          # send patch 3000 only

>        hg email -r 3000 -r 3001  # send patches 3000 and 3001 @@ -551,11

> +555,14 @@ def patchbomb(ui, repo, *revs, **opts):

>          raise util.Abort(_('no recipient addresses provided'))

>      cc = getaddrs('Cc', ask=True, default='') or []

>      bcc = getaddrs('Bcc') or []

>      replyto = getaddrs('Reply-To')

> 

> -    if opts.get('diffstat') or opts.get('confirm'):

> +    confirm = ui.configbool('patchbomb', 'confirm')

> +    confirm |= bool(opts.get('diffstat') or opts.get('confirm'))

> +

> +    if confirm:

>          ui.write(_('\nFinal summary:\n\n'), label='patchbomb.finalsummary')

>          ui.write(('From: %s\n' % sender), label='patchbomb.from')

>          for addr in showaddrs:

>              ui.write('%s\n' % addr, label='patchbomb.to')

>          for m, subj, ds in msgs:

> diff --git a/tests/test-patchbomb.t b/tests/test-patchbomb.t

> --- a/tests/test-patchbomb.t

> +++ b/tests/test-patchbomb.t

> @@ -84,10 +84,30 @@ Mercurial-patchbomb/.* -> Mercurial-patc

> 

>    are you sure you want to send (yn)? n

>    abort: patchbomb canceled

>    [255]

> 

> +  $ hg --config ui.interactive=1 --config patchbomb.confirm=true email

> + -n -f quux -t foo -c bar -r tip<<EOF  > n  > EOF  this patch series

> + consists of 1 patches.

> +

> +

> +  Final summary:

> +

> +  From: quux

> +  To: foo

> +  Cc: bar

> +  Subject: [PATCH] a

> +   a |  1 +

> +   1 files changed, 1 insertions(+), 0 deletions(-)

> +

> +  are you sure you want to send (yn)? n

> +  abort: patchbomb canceled

> +  [255]

> +

> +

>  Test diff.git is respected

>    $ hg --config diff.git=True email --date '1970-1-1 0:1' -n -f quux -t foo -c bar -

> r tip

>    this patch series consists of 1 patches.

> 

> 

> _______________________________________________

> Mercurial-devel mailing list

> Mercurial-devel@selenic.com

> http://selenic.com/mailman/listinfo/mercurial-devel
Simon King - Dec. 3, 2014, 10:22 a.m.
On Wed, Dec 3, 2014 at 3:12 AM, Pierre-Yves David
<pierre-yves.david@ens-lyon.org> wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1417570521 28800
> #      Tue Dec 02 17:35:21 2014 -0800
> # Node ID 20e66de4913566197f0066f5835086c844deb401
> # Parent  d943db1f1f572bf3c01b8ca6b5e728ebc30649aa
> patchbomb: introduce a 'patchwork.confirm' option

typo: patchbomb.confirm, not patchwork ;-)

>
> When set to true, this option will make patchbomb always ask for confirmation
> before sending the email. Confirmation is a powerful way to prevent stupid
> mistake when the sending patches.
>
> This should let me get ride of my global alias adding
> --confirm to hg email.
>
> I know that some people may get bitten when moving from a machine with confirm
> configured  to machine where it is not. But I think it is still worth the risk.
>
> diff --git a/hgext/patchbomb.py b/hgext/patchbomb.py
> --- a/hgext/patchbomb.py
> +++ b/hgext/patchbomb.py
> @@ -50,10 +50,13 @@ overwritten by command line flags like -
>
>    [patchbomb]
>    intro=auto   # include introduction message if more than 1 patch (default)
>    intro=never  # never included an introduction message
>    intro=always # always include an introduction message
> +
> +You can set patchbomb to always ask for confirmation by setting
> +``patchbomb.confirm`` to true.
>  '''
>
>  import os, errno, socket, tempfile, cStringIO
>  import email
>  # On python2.4 you have to import these by name or they fail to
> @@ -428,13 +431,14 @@ def patchbomb(ui, repo, *revs, **opts):
>      Then when all is done, patchbomb messages are displayed. If the
>      PAGER environment variable is set, your pager will be fired up once
>      for each patchbomb message, so you can verify everything is alright.
>
>      In case email sending fails, you will find a backup of your series
> -    introductory message in ``.hg/last-email.txt``. The inclusion the
> -    introduction can also be control using the ``patchbomb.intro`` option. (see
> -    hg help patchbomb for details)
> +    introductory message in ``.hg/last-email.txt``.
> +
> +    The default behavior of this command can be customized through
> +    configuration. (See :hg:`help patchbomb` for details)
>
>      Examples::
>
>        hg email -r 3000          # send patch 3000 only
>        hg email -r 3000 -r 3001  # send patches 3000 and 3001
> @@ -551,11 +555,14 @@ def patchbomb(ui, repo, *revs, **opts):
>          raise util.Abort(_('no recipient addresses provided'))
>      cc = getaddrs('Cc', ask=True, default='') or []
>      bcc = getaddrs('Bcc') or []
>      replyto = getaddrs('Reply-To')
>
> -    if opts.get('diffstat') or opts.get('confirm'):
> +    confirm = ui.configbool('patchbomb', 'confirm')
> +    confirm |= bool(opts.get('diffstat') or opts.get('confirm'))
> +
> +    if confirm:
>          ui.write(_('\nFinal summary:\n\n'), label='patchbomb.finalsummary')
>          ui.write(('From: %s\n' % sender), label='patchbomb.from')
>          for addr in showaddrs:
>              ui.write('%s\n' % addr, label='patchbomb.to')
>          for m, subj, ds in msgs:
> diff --git a/tests/test-patchbomb.t b/tests/test-patchbomb.t
> --- a/tests/test-patchbomb.t
> +++ b/tests/test-patchbomb.t
> @@ -84,10 +84,30 @@ Mercurial-patchbomb/.* -> Mercurial-patc
>
>    are you sure you want to send (yn)? n
>    abort: patchbomb canceled
>    [255]
>
> +  $ hg --config ui.interactive=1 --config patchbomb.confirm=true email -n -f quux -t foo -c bar -r tip<<EOF
> +  > n
> +  > EOF
> +  this patch series consists of 1 patches.
> +
> +
> +  Final summary:
> +
> +  From: quux
> +  To: foo
> +  Cc: bar
> +  Subject: [PATCH] a
> +   a |  1 +
> +   1 files changed, 1 insertions(+), 0 deletions(-)
> +
> +  are you sure you want to send (yn)? n
> +  abort: patchbomb canceled
> +  [255]
> +
> +
>  Test diff.git is respected
>    $ hg --config diff.git=True email --date '1970-1-1 0:1' -n -f quux -t foo -c bar -r tip
>    this patch series consists of 1 patches.
>
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Mads Kiilerich - Dec. 3, 2014, 1:26 p.m.
On 12/03/2014 04:12 AM, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1417570521 28800
> #      Tue Dec 02 17:35:21 2014 -0800
> # Node ID 20e66de4913566197f0066f5835086c844deb401
> # Parent  d943db1f1f572bf3c01b8ca6b5e728ebc30649aa
> patchbomb: introduce a 'patchwork.confirm' option
>
> When set to true, this option will make patchbomb always ask for confirmation
> before sending the email. Confirmation is a powerful way to prevent stupid
> mistake when the sending patches.
>
> This should let me get ride of my global alias adding
> --confirm to hg email.
>
> I know that some people may get bitten when moving from a machine with confirm
> configured  to machine where it is not. But I think it is still worth the risk.

I would certainly enable this and take the risk of not minimizing the 
risk everywhere.

Somewhat related: I would also like to add a similar option to push, 
kind of like 'hg outgoing && hg bundle && echo bundle info && read ok && 
hg push' but using the same discovery and bundle for all operations.

/Mads
Jordi GutiƩrrez Hermoso - Dec. 3, 2014, 2:25 p.m.
On Tue, 2014-12-02 at 19:12 -0800, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1417570521 28800
> #      Tue Dec 02 17:35:21 2014 -0800
> # Node ID 20e66de4913566197f0066f5835086c844deb401
> # Parent  d943db1f1f572bf3c01b8ca6b5e728ebc30649aa
> patchbomb: introduce a 'patchwork.confirm' option
> 
> When set to true, this option will make patchbomb always ask for
> confirmation before sending the email. Confirmation is a powerful
> way to prevent stupid mistake when the sending patches.

Isn't it pretty simple to just alias `email` to `email --confirm` ?
That's what I did. I don't think we need a new config option.
Pierre-Yves David - Dec. 4, 2014, 2:42 p.m.
On 12/03/2014 02:22 AM, Simon King wrote:
> On Wed, Dec 3, 2014 at 3:12 AM, Pierre-Yves David
> <pierre-yves.david@ens-lyon.org> wrote:
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david@fb.com>
>> # Date 1417570521 28800
>> #      Tue Dec 02 17:35:21 2014 -0800
>> # Node ID 20e66de4913566197f0066f5835086c844deb401
>> # Parent  d943db1f1f572bf3c01b8ca6b5e728ebc30649aa
>> patchbomb: introduce a 'patchwork.confirm' option
>
> typo: patchbomb.confirm, not patchwork ;-)


Ok, I've not seen any functional feedback on this series.

There is two typos (2: patchnbomb; 3: pathwork) it sounds like something 
fixable in flight but I can send a V2 is necessary.
Augie Fackler - Dec. 5, 2014, 8:51 p.m.
On Tue, Dec 02, 2014 at 07:12:06PM -0800, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1417570521 28800
> #      Tue Dec 02 17:35:21 2014 -0800
> # Node ID 20e66de4913566197f0066f5835086c844deb401
> # Parent  d943db1f1f572bf3c01b8ca6b5e728ebc30649aa
> patchbomb: introduce a 'patchwork.confirm' option

Queued these, thanks.

>
> When set to true, this option will make patchbomb always ask for confirmation
> before sending the email. Confirmation is a powerful way to prevent stupid
> mistake when the sending patches.
>
> This should let me get ride of my global alias adding
> --confirm to hg email.
>
> I know that some people may get bitten when moving from a machine with confirm
> configured  to machine where it is not. But I think it is still worth the risk.
>
> diff --git a/hgext/patchbomb.py b/hgext/patchbomb.py
> --- a/hgext/patchbomb.py
> +++ b/hgext/patchbomb.py
> @@ -50,10 +50,13 @@ overwritten by command line flags like -
>
>    [patchbomb]
>    intro=auto   # include introduction message if more than 1 patch (default)
>    intro=never  # never included an introduction message
>    intro=always # always include an introduction message
> +
> +You can set patchbomb to always ask for confirmation by setting
> +``patchbomb.confirm`` to true.
>  '''
>
>  import os, errno, socket, tempfile, cStringIO
>  import email
>  # On python2.4 you have to import these by name or they fail to
> @@ -428,13 +431,14 @@ def patchbomb(ui, repo, *revs, **opts):
>      Then when all is done, patchbomb messages are displayed. If the
>      PAGER environment variable is set, your pager will be fired up once
>      for each patchbomb message, so you can verify everything is alright.
>
>      In case email sending fails, you will find a backup of your series
> -    introductory message in ``.hg/last-email.txt``. The inclusion the
> -    introduction can also be control using the ``patchbomb.intro`` option. (see
> -    hg help patchbomb for details)
> +    introductory message in ``.hg/last-email.txt``.
> +
> +    The default behavior of this command can be customized through
> +    configuration. (See :hg:`help patchbomb` for details)
>
>      Examples::
>
>        hg email -r 3000          # send patch 3000 only
>        hg email -r 3000 -r 3001  # send patches 3000 and 3001
> @@ -551,11 +555,14 @@ def patchbomb(ui, repo, *revs, **opts):
>          raise util.Abort(_('no recipient addresses provided'))
>      cc = getaddrs('Cc', ask=True, default='') or []
>      bcc = getaddrs('Bcc') or []
>      replyto = getaddrs('Reply-To')
>
> -    if opts.get('diffstat') or opts.get('confirm'):
> +    confirm = ui.configbool('patchbomb', 'confirm')
> +    confirm |= bool(opts.get('diffstat') or opts.get('confirm'))
> +
> +    if confirm:
>          ui.write(_('\nFinal summary:\n\n'), label='patchbomb.finalsummary')
>          ui.write(('From: %s\n' % sender), label='patchbomb.from')
>          for addr in showaddrs:
>              ui.write('%s\n' % addr, label='patchbomb.to')
>          for m, subj, ds in msgs:
> diff --git a/tests/test-patchbomb.t b/tests/test-patchbomb.t
> --- a/tests/test-patchbomb.t
> +++ b/tests/test-patchbomb.t
> @@ -84,10 +84,30 @@ Mercurial-patchbomb/.* -> Mercurial-patc
>
>    are you sure you want to send (yn)? n
>    abort: patchbomb canceled
>    [255]
>
> +  $ hg --config ui.interactive=1 --config patchbomb.confirm=true email -n -f quux -t foo -c bar -r tip<<EOF
> +  > n
> +  > EOF
> +  this patch series consists of 1 patches.
> +
> +
> +  Final summary:
> +
> +  From: quux
> +  To: foo
> +  Cc: bar
> +  Subject: [PATCH] a
> +   a |  1 +
> +   1 files changed, 1 insertions(+), 0 deletions(-)
> +
> +  are you sure you want to send (yn)? n
> +  abort: patchbomb canceled
> +  [255]
> +
> +
>  Test diff.git is respected
>    $ hg --config diff.git=True email --date '1970-1-1 0:1' -n -f quux -t foo -c bar -r tip
>    this patch series consists of 1 patches.
>
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/hgext/patchbomb.py b/hgext/patchbomb.py
--- a/hgext/patchbomb.py
+++ b/hgext/patchbomb.py
@@ -50,10 +50,13 @@  overwritten by command line flags like -
 
   [patchbomb]
   intro=auto   # include introduction message if more than 1 patch (default)
   intro=never  # never included an introduction message
   intro=always # always include an introduction message
+
+You can set patchbomb to always ask for confirmation by setting
+``patchbomb.confirm`` to true.
 '''
 
 import os, errno, socket, tempfile, cStringIO
 import email
 # On python2.4 you have to import these by name or they fail to
@@ -428,13 +431,14 @@  def patchbomb(ui, repo, *revs, **opts):
     Then when all is done, patchbomb messages are displayed. If the
     PAGER environment variable is set, your pager will be fired up once
     for each patchbomb message, so you can verify everything is alright.
 
     In case email sending fails, you will find a backup of your series
-    introductory message in ``.hg/last-email.txt``. The inclusion the
-    introduction can also be control using the ``patchbomb.intro`` option. (see
-    hg help patchbomb for details)
+    introductory message in ``.hg/last-email.txt``.
+
+    The default behavior of this command can be customized through
+    configuration. (See :hg:`help patchbomb` for details)
 
     Examples::
 
       hg email -r 3000          # send patch 3000 only
       hg email -r 3000 -r 3001  # send patches 3000 and 3001
@@ -551,11 +555,14 @@  def patchbomb(ui, repo, *revs, **opts):
         raise util.Abort(_('no recipient addresses provided'))
     cc = getaddrs('Cc', ask=True, default='') or []
     bcc = getaddrs('Bcc') or []
     replyto = getaddrs('Reply-To')
 
-    if opts.get('diffstat') or opts.get('confirm'):
+    confirm = ui.configbool('patchbomb', 'confirm')
+    confirm |= bool(opts.get('diffstat') or opts.get('confirm'))
+
+    if confirm:
         ui.write(_('\nFinal summary:\n\n'), label='patchbomb.finalsummary')
         ui.write(('From: %s\n' % sender), label='patchbomb.from')
         for addr in showaddrs:
             ui.write('%s\n' % addr, label='patchbomb.to')
         for m, subj, ds in msgs:
diff --git a/tests/test-patchbomb.t b/tests/test-patchbomb.t
--- a/tests/test-patchbomb.t
+++ b/tests/test-patchbomb.t
@@ -84,10 +84,30 @@  Mercurial-patchbomb/.* -> Mercurial-patc
   
   are you sure you want to send (yn)? n
   abort: patchbomb canceled
   [255]
 
+  $ hg --config ui.interactive=1 --config patchbomb.confirm=true email -n -f quux -t foo -c bar -r tip<<EOF
+  > n
+  > EOF
+  this patch series consists of 1 patches.
+  
+  
+  Final summary:
+  
+  From: quux
+  To: foo
+  Cc: bar
+  Subject: [PATCH] a
+   a |  1 +
+   1 files changed, 1 insertions(+), 0 deletions(-)
+  
+  are you sure you want to send (yn)? n
+  abort: patchbomb canceled
+  [255]
+
+
 Test diff.git is respected
   $ hg --config diff.git=True email --date '1970-1-1 0:1' -n -f quux -t foo -c bar -r tip
   this patch series consists of 1 patches.