Hi Dave,
PFA updated patch.
On Mon, Jan 29, 2018 at 3:23 PM, Dave Page <[email protected]> wrote:
> Hi
>
> On Sun, Jan 28, 2018 at 4:42 AM, Murtuza Zabuawala <murtuza.zabuawala@
> enterprisedb.com> 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)?
>
Yes, we can add but I thought that it would be good if we have all the
requirements at one place instead new developers/contributors having to go
and search into separate directory for development/test related
dependencies.
>
>
>>
>>
>>> - 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
>
diff --git a/web/.pycodestyle b/web/.pycodestyle
new file mode 100644
index 0000000..ec01c1a
--- /dev/null
+++ b/web/.pycodestyle
@@ -0,0 +1,9 @@
+[pycodestyle]
+ignore = E402
+max-line-length = 79
+statistics = True
+show-source = False
+show-pep8 = False
+count = True
+format = pylint
+exclude = migrations,node_modules
diff --git a/web/package.json b/web/package.json
index 43f96ca..8542797 100644
--- a/web/package.json
+++ b/web/package.json
@@ -95,6 +95,7 @@
"test:karma-once": "yarn run linter && yarn run karma start --single-run",
"test:karma": "yarn run linter && yarn run karma start",
"test:feature": "yarn run bundle && python regression/runtests.py --pkg
feature_tests",
- "test": "yarn run test:karma-once && yarn run bundle && python
regression/runtests.py"
+ "test": "yarn run test:karma-once && yarn run bundle && python
regression/runtests.py",
+ "pep8": "pycodestyle --config=.pycodestyle ."
}
}
diff --git a/web/regression/requirements.txt b/web/regression/requirements.txt
index 69f4a27..cb93353 100644
--- a/web/regression/requirements.txt
+++ b/web/regression/requirements.txt
@@ -1,3 +1,12 @@
+#####################################
+# Include all the common dependencies
+#####################################
+-r ../../requirements.txt
+
+###########################################
+# Add all the development dependencies here
+###########################################
+pycodestyle==2.3.1
pyperclip~=1.5.27
selenium==3.3.3; python_version < '2.7'
selenium==3.7.0; python_version >= '2.7'
@@ -7,5 +16,6 @@ traceback2==1.4.0
unittest2==1.1.0
mock===2.0.0; python_version < '3.3'
-# Leave this at the end because there is a bug where the '--install-option' is
applied to all subsequent requirements
+# Leave this at the end because there is a bug where the '--install-option'
+# is applied to all subsequent requirements
chromedriver_installer==0.0.6 --install-option='--chromedriver-version=2.33'
