Hi On Sun, Jan 28, 2018 at 4:42 AM, Murtuza Zabuawala < [email protected]> wrote:
> Hi Dave, > > On Fri, Jan 26, 2018 at 10:33 PM, Dave Page <[email protected]> wrote: > >> Hi >> >> On Fri, Jan 26, 2018 at 1:26 PM, Murtuza Zabuawala < >> [email protected]> 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. > Can't we just add "-r ../../requirements.txt" to web/regression/requirements.txt? I can't imagine any situation in which we'd have a requirement for running that isn't needed for testing (or that we'd care about if we did)? > > >> - 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? > > We have targets in /Makefile for testing. We should add one for this, and ensure it's also called when running "make check". > > >> - 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 > Right. That definitely looks more useful now. > > ... >> ... >> ./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 >> > > -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
