On (10/12/15 13:36), Petr Cech wrote:
On 12/10/2015 01:27 PM, Lukas Slebodnik wrote:
>On (10/12/15 12:54), Petr Cech wrote:
>>On 11/23/2015 03:09 PM, Jakub Hrozek wrote:
>>>On Wed, Nov 11, 2015 at 12:26:41PM +0100, Jakub Hrozek wrote:
>>>>When Python3 was used, the tests used our own compat function for
>>>>checking sequences. It's better to stick to the standard-library
>>>>provided methods.
>>>>
>>>>More details are in the commit message..
>>>
>>>Sorry, the patch wasn't correct, a new one is attached.
>>>
>>
>>Hi Jakub,
>>
>>I have done review. Code looks good to me and I catched this:
>>
>>http://sssd-ci.duckdns.org/logs/job/34/36/summary.html
>>
>>I asked the CI list... ok, it is not connected to your patch, so:
>>
>CI passed but new pep8 warnings were introduced.
>We get rid of them in integration best.
>But we needn't introduce new in other python files.
>
>sh$ git diff HEAD~..HEAD | pep8 --diff
>./src/tests/pyhbac-test.py:17:1: E302 expected 2 blank lines, found 1
>./src/tests/pyhbac-test.py:20:1: E302 expected 2 blank lines, found 1
>./src/tests/pyhbac-test.py:39:1: E302 expected 2 blank lines, found 1
>./src/tests/pyhbac-test.py:75:1: E302 expected 2 blank lines, found 1
>./src/tests/pyhbac-test.py:83:18: E201 whitespace after '['
>./src/tests/pyhbac-test.py:83:31: E202 whitespace before ']'
>./src/tests/pyhbac-test.py:87:19: E201 whitespace after '['
>./src/tests/pyhbac-test.py:87:32: E202 whitespace before ']'
>./src/tests/pyhbac-test.py:92:18: E201 whitespace after '['
>./src/tests/pyhbac-test.py:92:31: E202 whitespace before ']'
>./src/tests/pyhbac-test.py:98:19: E201 whitespace after '['
>./src/tests/pyhbac-test.py:98:32: E202 whitespace before ']'
>./src/tests/pyhbac-test.py:105:19: E201 whitespace after '('
>./src/tests/pyhbac-test.py:105:32: E202 whitespace before ')'
>./src/tests/pyhbac-test.py:138:42: E201 whitespace after '['
>./src/tests/pyhbac-test.py:138:55: E202 whitespace before ']'
>./src/tests/pyhbac-test.py:139:43: E201 whitespace after '['
>./src/tests/pyhbac-test.py:139:56: E202 whitespace before ']'
>./src/tests/pyhbac-test.py:226:80: E501 line too long (90 > 79 characters)
>./src/tests/pyhbac-test.py:236:50: E201 whitespace after '['
>./src/tests/pyhbac-test.py:236:63: E202 whitespace before ']'
>./src/tests/pyhbac-test.py:237:51: E201 whitespace after '['
>./src/tests/pyhbac-test.py:237:64: E202 whitespace before ']'
>./src/tests/pyhbac-test.py:269:41: E201 whitespace after '('
>./src/tests/pyhbac-test.py:272:78: E202 whitespace before ')'
>./src/tests/pyhbac-test.py:272:80: E501 line too long (80 > 79 characters)
>./src/tests/pyhbac-test.py:279:41: E201 whitespace after '('
>./src/tests/pyhbac-test.py:280:78: E202 whitespace before ')'
>./src/tests/pyhbac-test.py:280:80: E501 line too long (80 > 79 characters)
>./src/tests/pyhbac-test.py:282:29: E201 whitespace after '['
>./src/tests/pyhbac-test.py:282:37: E202 whitespace before ']'
>./src/tests/pyhbac-test.py:288:1: E302 expected 2 blank lines, found 1
>./src/tests/pyhbac-test.py:299:19: E201 whitespace after '['
>./src/tests/pyhbac-test.py:299:32: E202 whitespace before ']'
>./src/tests/pyhbac-test.py:310:19: E201 whitespace after '['
>./src/tests/pyhbac-test.py:310:32: E202 whitespace before ']'
>./src/tests/pyhbac-test.py:317:19: E201 whitespace after '('
>./src/tests/pyhbac-test.py:317:32: E202 whitespace before ')'
>./src/tests/pysss_murmur-test.py:29:28: E261 at least two spaces before inline
comment
>./src/tests/pysss_murmur-test.py:29:29: E262 inline comment should start with '#
'
>./src/tests/pysss_murmur-test.py:31:1: E302 expected 2 blank lines, found 1
>
>LS
Well, that seems to be valid point. I take back my ACK.
Lukas, I see that you run PEP8 locally. Is there plan to add PEP8 to CI?
we have many pep8 warnings in old python code
https://fedorahosted.org/sssd/ticket/2774
that's the reason why I shared command how to checked newly introduced
warnings.
git diff HEAD~..HEAD | pep8 --diff
Feel free to integrate this command to CI.
LS