Patchwork [2,of,2,v2] fsmonitor: refresh pywatchman to upstream

login
register
mail settings
Submitter Pierre-Yves David
Date Dec. 22, 2016, 10:56 a.m.
Message ID <550e4ed2-5887-0e08-b8a3-b29d06e3cf2f@ens-lyon.org>
Download mbox | patch
Permalink /patch/18005/
State Not Applicable
Headers show

Comments

Pierre-Yves David - Dec. 22, 2016, 10:56 a.m.
On 12/21/2016 11:05 PM, Zack Hricz via Mercurial-devel wrote:
> # HG changeset patch
> # User zphricz <zack.hricz@oculus.com>
> # Date 1482357422 28800
> #      Wed Dec 21 13:57:02 2016 -0800
> # Node ID 2176ee51fcde4fda4782f6835d987ae9a6d6a3f0
> # Parent  a3d289ca22c1066bc929bb519b655fce9fd39bbe
> fsmonitor: refresh pywatchman to upstream
>
> Update to upstream to version c77452 with additional fixes to tests.
> The refresh includes fixes to improve windows compatibility.
> # no-check-commit

This series looks fine but test-check-pyflakes complains about it the 
second patch. I've pushed the first patch.


Can you get that fixed upstream (alongside the small import fixes)

assigned to but never used
    tests/filterpyflakes.py:39: undefined name 'undefinedname'

ERROR: test-check-pyflakes.t output changed

You can also drop the inserted:

   "no-check-code -- this is a 3rd party library"

since David Soria Para already made the whole directory ignored.
via Mercurial-devel - Dec. 22, 2016, 6:29 p.m.
> On 12/21/2016 11:05 PM, Zack Hricz via Mercurial-devel wrote:

> > # HG changeset patch

> > # User zphricz <zack.hricz@oculus.com> # Date 1482357422 28800

> > #      Wed Dec 21 13:57:02 2016 -0800

> > # Node ID 2176ee51fcde4fda4782f6835d987ae9a6d6a3f0

> > # Parent  a3d289ca22c1066bc929bb519b655fce9fd39bbe

> > fsmonitor: refresh pywatchman to upstream

> >

> > Update to upstream to version c77452 with additional fixes to tests.

> > The refresh includes fixes to improve windows compatibility.

> > # no-check-commit

> 

> This series looks fine but test-check-pyflakes complains about it the second

> patch. I've pushed the first patch.

> 

> 

> Can you get that fixed upstream (alongside the small import fixes)

> 

> --- /home/marmoute/mercurial-testing/tests/test-check-pyflakes.t

> +++ /home/marmoute/mercurial-testing/tests/test-check-pyflakes.t.err

> @@ -11,5 +11,10 @@

>     > 2>/dev/null \

>     > | xargs pyflakes 2>/dev/null | "$TESTDIR/filterpyflakes.py"

>     contrib/python-zstandard/tests/test_data_structures.py:107: local

> variable 'size' is assigned to but never used

> +  hgext/fsmonitor/pywatchman/__init__.py:51: 'load' imported but unused

> +  hgext/fsmonitor/pywatchman/capabilities.py:34: 're' imported but unused

> +  hgext/fsmonitor/pywatchman/capabilities.py:65: local variable

> 'failed' is assigned to but never used

> +  hgext/fsmonitor/pywatchman/pybser.py:38: 'sys' imported but unused

> +  hgext/fsmonitor/pywatchman/pybser.py:270: local variable 'ex' is

> assigned to but never used

>     tests/filterpyflakes.py:39: undefined name 'undefinedname'

> 

> ERROR: test-check-pyflakes.t output changed

> 

> You can also drop the inserted:

> 

>    "no-check-code -- this is a 3rd party library"

> 

> since David Soria Para already made the whole directory ignored.


Both test-check-code and test-check-pyflakes skip over files with 
"no-check-code" in it. Now that test-check-code skips over everything 
under pywatchman, would it not make sense to do the same for 
test-check-pyflakes?

-Zack
Pierre-Yves David - Dec. 22, 2016, 6:45 p.m.
On 12/22/2016 07:29 PM, Zack Hricz wrote:
>> On 12/21/2016 11:05 PM, Zack Hricz via Mercurial-devel wrote:
>>> # HG changeset patch
>>> # User zphricz <zack.hricz@oculus.com> # Date 1482357422 28800
>>> #      Wed Dec 21 13:57:02 2016 -0800
>>> # Node ID 2176ee51fcde4fda4782f6835d987ae9a6d6a3f0
>>> # Parent  a3d289ca22c1066bc929bb519b655fce9fd39bbe
>>> fsmonitor: refresh pywatchman to upstream
>>>
>>> Update to upstream to version c77452 with additional fixes to tests.
>>> The refresh includes fixes to improve windows compatibility.
>>> # no-check-commit
>>
>> This series looks fine but test-check-pyflakes complains about it the second
>> patch. I've pushed the first patch.
>>
>>
>> Can you get that fixed upstream (alongside the small import fixes)
>>
>> --- /home/marmoute/mercurial-testing/tests/test-check-pyflakes.t
>> +++ /home/marmoute/mercurial-testing/tests/test-check-pyflakes.t.err
>> @@ -11,5 +11,10 @@
>>     > 2>/dev/null \
>>     > | xargs pyflakes 2>/dev/null | "$TESTDIR/filterpyflakes.py"
>>     contrib/python-zstandard/tests/test_data_structures.py:107: local
>> variable 'size' is assigned to but never used
>> +  hgext/fsmonitor/pywatchman/__init__.py:51: 'load' imported but unused
>> +  hgext/fsmonitor/pywatchman/capabilities.py:34: 're' imported but unused
>> +  hgext/fsmonitor/pywatchman/capabilities.py:65: local variable
>> 'failed' is assigned to but never used
>> +  hgext/fsmonitor/pywatchman/pybser.py:38: 'sys' imported but unused
>> +  hgext/fsmonitor/pywatchman/pybser.py:270: local variable 'ex' is
>> assigned to but never used
>>     tests/filterpyflakes.py:39: undefined name 'undefinedname'
>>
>> ERROR: test-check-pyflakes.t output changed
>>
>> You can also drop the inserted:
>>
>>    "no-check-code -- this is a 3rd party library"
>>
>> since David Soria Para already made the whole directory ignored.
>
> Both test-check-code and test-check-pyflakes skip over files with
> "no-check-code" in it. Now that test-check-code skips over everything
> under pywatchman, would it not make sense to do the same for
> test-check-pyflakes?

Ha yes, we should have a consistent behavior between the file level 
exclusion and the directory level exclusion. Can you make them 
consistent and submit a V2 ?

(note: that would probably still be a good idea to fix these warning 
upstream)

Cheers,

Patch

--- /home/marmoute/mercurial-testing/tests/test-check-pyflakes.t
+++ /home/marmoute/mercurial-testing/tests/test-check-pyflakes.t.err
@@ -11,5 +11,10 @@ 
    > 2>/dev/null \
    > | xargs pyflakes 2>/dev/null | "$TESTDIR/filterpyflakes.py"
    contrib/python-zstandard/tests/test_data_structures.py:107: local 
variable 'size' is assigned to but never used
+  hgext/fsmonitor/pywatchman/__init__.py:51: 'load' imported but unused
+  hgext/fsmonitor/pywatchman/capabilities.py:34: 're' imported but unused
+  hgext/fsmonitor/pywatchman/capabilities.py:65: local variable 
'failed' is assigned to but never used
+  hgext/fsmonitor/pywatchman/pybser.py:38: 'sys' imported but unused
+  hgext/fsmonitor/pywatchman/pybser.py:270: local variable 'ex' is