Re: [pgAdmin4][RM#3063] Add 'pycodestyle ' Python PEP-8 checker module

Hi Dave,

On Fri, Jan 26, 2018 at 10:33 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

> Hi
>
> On Fri, Jan 26, 2018 at 1:26 PM, Murtuza Zabuawala <
> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>
>> Hi,
>>
>> PFA patch to add 'pycodestyle ' (formerly called pep8) which is a Python
>> PEP-8 checker module.
>>
>
> Nice!
>
> A couple of thoughts:
>
> - I think this should go into web/regression/requirements.txt. Otherwise,
> it'll end up being installed in the packages we build.
>

​I have come up with new design for requirements (Patch attached).

We can have layered requirement files, f
or example, we
​can have all the common dependancies in one file​
​say ​
requirements
/​
common.tx
​t, then​

​we can have ​
requirements
/​
​
requirements
​-​
dev
.tx
​t which will inherit all the common dependancies from
​
common.tx
​t plus it has its own development related
dependancies
​, similar with ​
requirements
/​
​
requirements.tx
t
which will inherit all the common dependancies from
​
common.tx
​t plus it has its own production related
dependancies
​(if any),

Then we can use pip install -r ​
requirements
​/
requirements
​-​
dev.txt​

​for the development ​
​and
​
pip install -r ​
requirements
​/requirements.txt​

​for the production ​
accordingly
​ without interfering between them.​

> - We should probably include a new Makefile target to run it, as well as
> including it in the "check" target.
>
​Sorry, I didn't get you on this one, could you please throw some light on
this?​

> - Once we expect the output to be clean, I think we should add a runner
> script to ci/ so we automate checking.
>
​Yes, we should
​.
​

> - Would a shorter output be more useful? In particular, it seems to be
> outputting a paragraph of text every time if finds a line longer than 79
> chars. Really, we just need the location of each issue, and then the
> summary I think, e.g.
>
> ​Done.
FYI you can configure it via show-source (displays faulty code) & show-pep8
(displays
​corrective

​suggestions
)
​ ​
option
​s​
in web/.pycodestyle

...
> ...
> ./pgadmin/utils/tests/test_versioned_template_loader.py:127: [E501] line
> too long (104 > 79 characters)
> ./pgadmin/utils/tests/test_versioned_template_loader.py:118: [E501] line
> too long (89 > 79 characters)
> ./pgadmin/utils/tests/test_versioned_template_loader.py:116: [E501] line
> too long (89 > 79 characters)
> 3 E111 indentation is not a multiple of four
> 52 E121 continuation line under-indented for hanging indent
> 19 E122 continuation line missing indentation or outdented
> 27 E123 closing bracket does not match indentation of opening
> bracket's line
> 3 E124 closing bracket does not match visual indentation
> 6 E125 continuation line with same indent as next logical line
> 79 E126 continuation line over-indented for hanging indent
> 67 E127 continuation line over-indented for visual indent
> 15 E128 continuation line under-indented for visual indent
> 3 E129 visually indented line with same indent as next logical line
> 11 E131 continuation line unaligned for hanging indent
> 6 E203 whitespace before ','
> 1 E211 whitespace before '['
> 2 E221 multiple spaces before operator
> 1 E222 multiple spaces after operator
> 19 E225 missing whitespace around operator
> 19 E226 missing whitespace around arithmetic operator
> 16 E231 missing whitespace after ','
> 2 E241 multiple spaces after ','
> 7 E251 unexpected spaces around keyword / parameter equals
> 1 E261 at least two spaces before inline comment
> 1 E271
> <https://maps.google.com/?q=1%C2%A0+%C2%A0+%C2%A0+%C2%A0E271&entry=gmail&source=g>
> multiple spaces after keyword
> 17 E302 expected 2 blank lines, found 1
> 23 E303 too many blank lines (2)
> 14 E305 expected 2 blank lines after class or function definition,
> found 1
> 1 E401 multiple imports on one line
> 1176 E501 line too long (80 > 79 characters)
> 15 E502 the backslash is redundant between brackets
> 4 E703 statement ends with a semicolon
> 8 E712
> <https://maps.google.com/?q=8%C2%A0+%C2%A0+%C2%A0+%C2%A0E712&entry=gmail&source=g>
> comparison to False should be 'if cond is False:' or 'if not cond:'
> 3 E713 test for membership should be 'not in'
> 21 E722 do not use bare except'
> 1 E741 ambiguous variable name 'l'
> 11 W391 blank line at end of file
> 23 W503 line break before binary operator
> 1677
>
> Thoughts?
>

​Please review.
​

>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>