Bug 199870

Summary:

check-webkit-style: Add limited Python3 support

Product:

WebKit

Reporter:

Jonathan Bedard <jbedard>

Component:

Tools / Tests

Assignee:

Jonathan Bedard <jbedard>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

aakash_jain, ap, commit-queue, dean_johnson, dewei_zhu, ews-watchlist, glenn, mcatanzaro, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none

Jonathan Bedard

Reported 2019-07-17 10:51:44 PDT

Pylint isn't practical (since check-webkit-style is Python 2, any Python 3 syntax will fail), but pycodestyle (the replacement for pep8) is.

Attachments
Patch (6.61 KB, patch) 2019-07-17 11:09 PDT, Jonathan Bedard	no flags	Details Formatted Diff Diff
Patch (6.46 KB, patch) 2019-07-17 13:00 PDT, Jonathan Bedard	no flags	Details Formatted Diff Diff
Patch (6.47 KB, patch) 2019-07-17 13:27 PDT, Jonathan Bedard	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Jonathan Bedard

Comment 1 2019-07-17 11:09:21 PDT

I also think that our current version of pep8 doesn't support Python 3.

Jonathan Bedard

Comment 2 2019-07-17 11:09:33 PDT

Created attachment 374309 [details] Patch

Dean Johnson

Comment 3 2019-07-17 11:16:48 PDT

Comment on attachment 374309 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=374309&action=review Looks mostly good to me, but I am curious about the categories missing for pycodestyle/W191 and pycodestyle/W291. > Tools/Scripts/webkitpy/style/checker.py:428 > + categories = categories.union(["pep8/W191", "pep8/W291", "pep8/E501", "pycodestyle/E501"]) Any reason not to add pycodestyle/W191 and pycodestyle/W291? > Tools/Scripts/webkitpy/style/checkers/python.py:32 > +from webkitpy.thirdparty.autoinstalled import pycodestyle Maybe we should put this import into `def check` so it isn't installed for everyone who isn't trying to check python3 code style? > Tools/Scripts/webkitpy/style/checkers/python.py:142 > + code = text[:4] Can you add comments or links to documentation for this? > Tools/Scripts/webkitpy/style/checkers/python.py:143 > + message = text[5:] Ditto. > Tools/Scripts/webkitpy/style/checkers/python.py:145 > + self._handle_style_error(line_number, category, 5, message) Ditto (on '5').

Jonathan Bedard

Comment 4 2019-07-17 11:27:53 PDT

Comment on attachment 374309 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=374309&action=review >> Tools/Scripts/webkitpy/style/checker.py:428 >> + categories = categories.union(["pep8/W191", "pep8/W291", "pep8/E501", "pycodestyle/E501"]) > > Any reason not to add pycodestyle/W191 and pycodestyle/W291? Because that's supporting Tools/WebGPUAPIStructure, which it looks like might have tabs in it. In short, we don't need it, and I'm trying not to inherit any more of webkitpy's technical debt than I have to.

Jonathan Bedard

Comment 5 2019-07-17 13:00:29 PDT

Created attachment 374318 [details] Patch

Aakash Jain

Comment 6 2019-07-17 13:08:15 PDT

rs=me

Jonathan Bedard

Comment 7 2019-07-17 13:27:10 PDT

Created attachment 374323 [details] Patch

WebKit Commit Bot

Comment 8 2019-07-17 14:40:50 PDT

Comment on attachment 374323 [details] Patch Clearing flags on attachment: 374323 Committed r247538: <https://trac.webkit.org/changeset/247538>