Patchwork run-tests: drop unused "useipv6" parameter from Test class

login
register
mail settings
Submitter via Mercurial-devel
Date Feb. 16, 2017, 6:20 a.m.
Message ID <b01c29d1eaff7875380b.1487226028@martinvonz.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/18545/
State Rejected
Headers show

Comments

via Mercurial-devel - Feb. 16, 2017, 6:20 a.m.
# HG changeset patch
# User Martin von Zweigbergk <martinvonz@google.com>
# Date 1487225871 28800
#      Wed Feb 15 22:17:51 2017 -0800
# Node ID b01c29d1eaff7875380b964da46b357d3a4445b5
# Parent  1ee685defe80117cf6aafea1ede6c33c478abceb
run-tests: drop unused "useipv6" parameter from Test class

The global parameter is used instead, so the constructor parameter
ended up being unused.
Jun Wu - Feb. 16, 2017, 6:28 a.m.
Looks good to me. I was trying to make the test class "self-contained"
without accessing global variables, as some people have strong opinion on
global variables. So it was intentional. But removing the parameter leads to
shorter code. I'm fine with either way.

Excerpts from Martin von Zweigbergk's message of 2017-02-15 22:20:28 -0800:
> # HG changeset patch
> # User Martin von Zweigbergk <martinvonz@google.com>
> # Date 1487225871 28800
> #      Wed Feb 15 22:17:51 2017 -0800
> # Node ID b01c29d1eaff7875380b964da46b357d3a4445b5
> # Parent  1ee685defe80117cf6aafea1ede6c33c478abceb
> run-tests: drop unused "useipv6" parameter from Test class
> 
> The global parameter is used instead, so the constructor parameter
> ended up being unused.
> 
> diff -r 1ee685defe80 -r b01c29d1eaff tests/run-tests.py
> --- a/tests/run-tests.py    Wed Feb 15 16:29:58 2017 -0800
> +++ b/tests/run-tests.py    Wed Feb 15 22:17:51 2017 -0800
> @@ -534,8 +534,7 @@
>                   timeout=defaults['timeout'],
>                   startport=defaults['port'], extraconfigopts=None,
>                   py3kwarnings=False, shell=None, hgcommand=None,
> -                 slowtimeout=defaults['slowtimeout'], usechg=False,
> -                 useipv6=False):
> +                 slowtimeout=defaults['slowtimeout'], usechg=False):
>          """Create a test from parameters.
>  
>          path is the full path to the file defining the test.
> @@ -2322,8 +2321,7 @@
>                      py3kwarnings=self.options.py3k_warnings,
>                      shell=self.options.shell,
>                      hgcommand=self._hgcommand,
> -                    usechg=bool(self.options.with_chg or self.options.chg),
> -                    useipv6=useipv6)
> +                    usechg=bool(self.options.with_chg or self.options.chg))
>          t.should_reload = True
>          return t
>
via Mercurial-devel - Feb. 16, 2017, 6:34 a.m.
On Wed, Feb 15, 2017 at 10:28 PM, Jun Wu <quark@fb.com> wrote:
> Looks good to me. I was trying to make the test class "self-contained"
> without accessing global variables, as some people have strong opinion on
> global variables. So it was intentional. But removing the parameter leads to
> shorter code. I'm fine with either way.

Oh, I'm sorry. I didn't realize it was shadowing the global one. I'm
still not sure which I prefer, but since my patch wasn't just dropping
an unused parameter as I thought it did, let's just drop my patch.

Patch

diff -r 1ee685defe80 -r b01c29d1eaff tests/run-tests.py
--- a/tests/run-tests.py	Wed Feb 15 16:29:58 2017 -0800
+++ b/tests/run-tests.py	Wed Feb 15 22:17:51 2017 -0800
@@ -534,8 +534,7 @@ 
                  timeout=defaults['timeout'],
                  startport=defaults['port'], extraconfigopts=None,
                  py3kwarnings=False, shell=None, hgcommand=None,
-                 slowtimeout=defaults['slowtimeout'], usechg=False,
-                 useipv6=False):
+                 slowtimeout=defaults['slowtimeout'], usechg=False):
         """Create a test from parameters.
 
         path is the full path to the file defining the test.
@@ -2322,8 +2321,7 @@ 
                     py3kwarnings=self.options.py3k_warnings,
                     shell=self.options.shell,
                     hgcommand=self._hgcommand,
-                    usechg=bool(self.options.with_chg or self.options.chg),
-                    useipv6=useipv6)
+                    usechg=bool(self.options.with_chg or self.options.chg))
         t.should_reload = True
         return t