Patchwork [2,of,6] check-code: always report when a file is skipped by "no-check-code"

login
register
mail settings
Submitter Simon Heimberg
Date Jan. 7, 2014, 9:31 p.m.
Message ID <661afe245eee6f739b52.1389130278@lapsi.heimberg.home>
Download mbox | patch
Permalink /patch/3272/
State Superseded
Commit 16b5f498f49cd606923b2d1cd77c7c5fb4cb7b38
Headers show

Comments

Simon Heimberg - Jan. 7, 2014, 9:31 p.m.
# HG changeset patch
# User Simon Heimberg <simohe@besonet.ch>
# Date 1389130155 -3600
#      Tue Jan 07 22:29:15 2014 +0100
# Node ID 661afe245eee6f739b52641878f87984baf06468
# Parent  0db2ed7afb31e2c6e2ca26c4f7ffbdf556759a15
check-code: always report when a file is skipped by "no-check-code"

Skipping an entire file generally from checking is an important event, so
report it always.
Do not tell the check name because skipping does not depend on it. Directly
skip the entire file instead of checking more patterns and skip again.

The pragma no-check-code was introduced by accident in the past. (Fixed in
e033a7d444ac and ee07f9d142c9.) This now is prevented because the files
to skip have to be listed in the test output of test-check-code-hg.t.
Pierre-Yves David - Jan. 8, 2014, 8:09 a.m.
On 01/07/2014 01:31 PM, Simon Heimberg wrote:
> # HG changeset patch
> # User Simon Heimberg <simohe@besonet.ch>
> # Date 1389130155 -3600
> #      Tue Jan 07 22:29:15 2014 +0100
> # Node ID 661afe245eee6f739b52641878f87984baf06468
> # Parent  0db2ed7afb31e2c6e2ca26c4f7ffbdf556759a15
> check-code: always report when a file is skipped by "no-check-code"
>
> Skipping an entire file generally from checking is an important event, so
> report it always.
> Do not tell the check name because skipping does not depend on it. Directly
> skip the entire file instead of checking more patterns and skip again.
>
> The pragma no-check-code was introduced by accident in the past. (Fixed in
> e033a7d444ac and ee07f9d142c9.) This now is prevented because the files
> to skip have to be listed in the test output of test-check-code-hg.t.
>
> diff -r 0db2ed7afb31 -r 661afe245eee contrib/check-code.py
> --- a/contrib/check-code.py	Tue Jan 07 22:28:45 2014 +0100
> +++ b/contrib/check-code.py	Tue Jan 07 22:29:15 2014 +0100
> @@ -442,10 +442,8 @@
>           pre = post = fp.read()
>           fp.close()
>           if "no-" "check-code" in pre:
> -            if debug:
> -                print "Skipping %s for %s it has no-" "check-code" % (
> -                       name, f)
> -            break
> +            print "Skipping %s it has no-" "check-code" % f
> +            return "Skip" # skip checking this file
>           for p, r in filters:
>               post = re.sub(p, r, post)
>           nerrs = len(pats[0]) # nerr elements are errors
> diff -r 0db2ed7afb31 -r 661afe245eee tests/test-check-code-hg.t
> --- a/tests/test-check-code-hg.t	Tue Jan 07 22:28:45 2014 +0100
> +++ b/tests/test-check-code-hg.t	Tue Jan 07 22:29:15 2014 +0100
> @@ -28,3 +28,8 @@
>   
>     $ { hg manifest 2>/dev/null; ls "$TESTTMP"/*.py | sed 's-\\-/-g'; } |
>     >   xargs "$check_code" --warnings --per-file=0 || false
> +  Skipping hgext/zeroconf/Zeroconf.py it has no-che?k-code (glob)
> +  Skipping i18n/polib.py it has no-che?k-code (glob)
> +  Skipping mercurial/httpclient/__init__.py it has no-che?k-code (glob)
> +  Skipping mercurial/httpclient/_readers.py it has no-che?k-code (glob)
> +  Skipping mercurial/httpclient/socketutil.py it has no-che?k-code (glob)

The no-che?k-code (glob) thing is here to prevent disabling of checkcode 
on test-check-code-hg.t ?
Simon Heimberg - Jan. 8, 2014, 11:40 a.m.
--On Mittwoch, 8. Januar 2014 00:09 -0800 Pierre-Yves David 
<pierre-yves.david@ens-lyon.org> wrote:

> On 01/07/2014 01:31 PM, Simon Heimberg wrote:
>> # HG changeset patch
>> # User Simon Heimberg <simohe@besonet.ch>
>> # Date 1389130155 -3600
>> #      Tue Jan 07 22:29:15 2014 +0100
>> # Node ID 661afe245eee6f739b52641878f87984baf06468
>> # Parent  0db2ed7afb31e2c6e2ca26c4f7ffbdf556759a15
>> check-code: always report when a file is skipped by "no-check-code"
>>
>> Skipping an entire file generally from checking is an important event, so
>> report it always.
>> Do not tell the check name because skipping does not depend on it.
>> Directly skip the entire file instead of checking more patterns and skip
>> again.
>>
>> The pragma no-check-code was introduced by accident in the past. (Fixed
>> in e033a7d444ac and ee07f9d142c9.) This now is prevented because the
>> files to skip have to be listed in the test output of
>> test-check-code-hg.t.
>>
>> diff -r 0db2ed7afb31 -r 661afe245eee contrib/check-code.py
>> --- a/contrib/check-code.py	Tue Jan 07 22:28:45 2014 +0100
>> +++ b/contrib/check-code.py	Tue Jan 07 22:29:15 2014 +0100
>> @@ -442,10 +442,8 @@
>>           pre = post = fp.read()
>>           fp.close()
>>           if "no-" "check-code" in pre:
>> -            if debug:
>> -                print "Skipping %s for %s it has no-" "check-code" % (
>> -                       name, f)
>> -            break
>> +            print "Skipping %s it has no-" "check-code" % f
>> +            return "Skip" # skip checking this file
>>           for p, r in filters:
>>               post = re.sub(p, r, post)
>>           nerrs = len(pats[0]) # nerr elements are errors
>> diff -r 0db2ed7afb31 -r 661afe245eee tests/test-check-code-hg.t
>> --- a/tests/test-check-code-hg.t	Tue Jan 07 22:28:45 2014 +0100
>> +++ b/tests/test-check-code-hg.t	Tue Jan 07 22:29:15 2014 +0100
>> @@ -28,3 +28,8 @@
>>
>>     $ { hg manifest 2>/dev/null; ls "$TESTTMP"/*.py | sed 's-\\-/-g'; } |
>>     >   xargs "$check_code" --warnings --per-file=0 || false
>> +  Skipping hgext/zeroconf/Zeroconf.py it has no-che?k-code (glob)
>> +  Skipping i18n/polib.py it has no-che?k-code (glob)
>> +  Skipping mercurial/httpclient/__init__.py it has no-che?k-code (glob)
>> +  Skipping mercurial/httpclient/_readers.py it has no-che?k-code (glob)
>> +  Skipping mercurial/httpclient/socketutil.py it has no-che?k-code
>> (glob)
>
> The no-che?k-code (glob) thing is here to prevent disabling of checkcode
> on test-check-code-hg.t ?

Exactly. I did not see any nice place to write this. Comments can not be in 
the
output. Maybe directly after would be a good place. (Only as long as there 
are
not many warnings. But this is discouraged, so it should not happen.) 
Should this
be done?
Pierre-Yves David - Jan. 8, 2014, 6:11 p.m.
On 01/08/2014 03:40 AM, Simon Heimberg wrote:
>
>
> --On Mittwoch, 8. Januar 2014 00:09 -0800 Pierre-Yves David 
> <pierre-yves.david@ens-lyon.org> wrote:
>
>> On 01/07/2014 01:31 PM, Simon Heimberg wrote:
>>> # HG changeset patch
>>> # User Simon Heimberg <simohe@besonet.ch>
>>> # Date 1389130155 -3600
>>> #      Tue Jan 07 22:29:15 2014 +0100
>>> # Node ID 661afe245eee6f739b52641878f87984baf06468
>>> # Parent  0db2ed7afb31e2c6e2ca26c4f7ffbdf556759a15
>>> check-code: always report when a file is skipped by "no-check-code"
>>>
>>> Skipping an entire file generally from checking is an important 
>>> event, so
>>> report it always.
>>> Do not tell the check name because skipping does not depend on it.
>>> Directly skip the entire file instead of checking more patterns and 
>>> skip
>>> again.
>>>
>>> The pragma no-check-code was introduced by accident in the past. (Fixed
>>> in e033a7d444ac and ee07f9d142c9.) This now is prevented because the
>>> files to skip have to be listed in the test output of
>>> test-check-code-hg.t.
>>>
>>> diff -r 0db2ed7afb31 -r 661afe245eee contrib/check-code.py
>>> --- a/contrib/check-code.py    Tue Jan 07 22:28:45 2014 +0100
>>> +++ b/contrib/check-code.py    Tue Jan 07 22:29:15 2014 +0100
>>> @@ -442,10 +442,8 @@
>>>           pre = post = fp.read()
>>>           fp.close()
>>>           if "no-" "check-code" in pre:
>>> -            if debug:
>>> -                print "Skipping %s for %s it has no-" "check-code" % (
>>> -                       name, f)
>>> -            break
>>> +            print "Skipping %s it has no-" "check-code" % f
>>> +            return "Skip" # skip checking this file
>>>           for p, r in filters:
>>>               post = re.sub(p, r, post)
>>>           nerrs = len(pats[0]) # nerr elements are errors
>>> diff -r 0db2ed7afb31 -r 661afe245eee tests/test-check-code-hg.t
>>> --- a/tests/test-check-code-hg.t    Tue Jan 07 22:28:45 2014 +0100
>>> +++ b/tests/test-check-code-hg.t    Tue Jan 07 22:29:15 2014 +0100
>>> @@ -28,3 +28,8 @@
>>>
>>>     $ { hg manifest 2>/dev/null; ls "$TESTTMP"/*.py | sed 
>>> 's-\\-/-g'; } |
>>>     >   xargs "$check_code" --warnings --per-file=0 || false
>>> +  Skipping hgext/zeroconf/Zeroconf.py it has no-che?k-code (glob)
>>> +  Skipping i18n/polib.py it has no-che?k-code (glob)
>>> +  Skipping mercurial/httpclient/__init__.py it has no-che?k-code 
>>> (glob)
>>> +  Skipping mercurial/httpclient/_readers.py it has no-che?k-code 
>>> (glob)
>>> +  Skipping mercurial/httpclient/socketutil.py it has no-che?k-code
>>> (glob)
>>
>> The no-che?k-code (glob) thing is here to prevent disabling of checkcode
>> on test-check-code-hg.t ?
>
> Exactly. I did not see any nice place to write this. Comments can not 
> be in the
> output. Maybe directly after would be a good place. (Only as long as 
> there are
> not many warnings. But this is discouraged, so it should not happen.) 
> Should this
> be done?

You can add your comment before the command, that would be fine.

Patch

diff -r 0db2ed7afb31 -r 661afe245eee contrib/check-code.py
--- a/contrib/check-code.py	Tue Jan 07 22:28:45 2014 +0100
+++ b/contrib/check-code.py	Tue Jan 07 22:29:15 2014 +0100
@@ -442,10 +442,8 @@ 
         pre = post = fp.read()
         fp.close()
         if "no-" "check-code" in pre:
-            if debug:
-                print "Skipping %s for %s it has no-" "check-code" % (
-                       name, f)
-            break
+            print "Skipping %s it has no-" "check-code" % f
+            return "Skip" # skip checking this file
         for p, r in filters:
             post = re.sub(p, r, post)
         nerrs = len(pats[0]) # nerr elements are errors
diff -r 0db2ed7afb31 -r 661afe245eee tests/test-check-code-hg.t
--- a/tests/test-check-code-hg.t	Tue Jan 07 22:28:45 2014 +0100
+++ b/tests/test-check-code-hg.t	Tue Jan 07 22:29:15 2014 +0100
@@ -28,3 +28,8 @@ 
 
   $ { hg manifest 2>/dev/null; ls "$TESTTMP"/*.py | sed 's-\\-/-g'; } |
   >   xargs "$check_code" --warnings --per-file=0 || false
+  Skipping hgext/zeroconf/Zeroconf.py it has no-che?k-code (glob)
+  Skipping i18n/polib.py it has no-che?k-code (glob)
+  Skipping mercurial/httpclient/__init__.py it has no-che?k-code (glob)
+  Skipping mercurial/httpclient/_readers.py it has no-che?k-code (glob)
+  Skipping mercurial/httpclient/socketutil.py it has no-che?k-code (glob)