Bug 179387 - Update code style guidelines for Python
Summary: Update code style guidelines for Python
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Website (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Aakash Jain
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-11-07 12:20 PST by Aakash Jain
Modified: 2018-06-27 10:06 PDT (History)
11 users (show)

See Also:


Attachments
Proposed patch (1.06 KB, patch)
2017-11-07 12:24 PST, Aakash Jain
ap: review+
Details | Formatted Diff | Diff
Archive of layout-test-results from ews106 for mac-elcapitan-wk2 (2.64 MB, application/zip)
2017-11-07 14:15 PST, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Aakash Jain 2017-11-07 12:20:17 PST
For Python code inside WebKit, we use PEP8 style. Recently, we had a discussion about it on webkit-dev and we decided to stick to PEP8. We should update the code style guidelines to reflect the same.
Comment 1 Aakash Jain 2017-11-07 12:24:20 PST
Created attachment 326238 [details]
Proposed patch
Comment 2 Alexey Proskuryakov 2017-11-07 14:12:41 PST
Comment on attachment 326238 [details]
Proposed patch

Is it worth mentioning that we don't respect the line length limit?
Comment 3 Build Bot 2017-11-07 14:15:43 PST
Comment on attachment 326238 [details]
Proposed patch

Attachment 326238 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/5138434

New failing tests:
imported/w3c/web-platform-tests/service-workers/service-worker/register-wait-forever-in-install-worker.https.html
Comment 4 Build Bot 2017-11-07 14:15:45 PST
Created attachment 326259 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 5 Aakash Jain 2017-11-07 14:22:19 PST
> Is it worth mentioning that we don't respect the line length limit?

I don't think we should mention it as a guideline. We don't want to encourage people to dis-respect line length. It's ok if they don't follow it, style checking script wouldn't complaint.
Comment 6 Alexey Proskuryakov 2018-01-24 14:41:30 PST
Comment on attachment 326238 [details]
Proposed patch

rs=me based on webkit-dev discussion
Comment 7 Aakash Jain 2018-01-24 14:48:55 PST
Committed r227576: <https://trac.webkit.org/changeset/227576>
Comment 8 Radar WebKit Bug Importer 2018-01-24 14:49:19 PST
<rdar://problem/36840858>
Comment 9 Daniel Bates 2018-04-03 20:23:56 PDT
(In reply to Aakash Jain from comment #5)
> > Is it worth mentioning that we don't respect the line length limit?
> 
> I don't think we should mention it as a guideline. We don't want to
> encourage people to dis-respect line length. 

I do not recall that we (the WebKit OpenSource Project) ever agreed to PEP-8's maximum line length rule. I certainly do not agree to it. It is archaic and hurts readability. These sentiments are echoed in the thread that includes <https://lists.webkit.org/pipermail/webkit-dev/2010-April/012486.html>.
Comment 10 Daniel Bates 2018-04-03 20:24:41 PDT
(In reply to Daniel Bates from comment #9)
> (In reply to Aakash Jain from comment #5)
> > > Is it worth mentioning that we don't respect the line length limit?
> > 
> > I don't think we should mention it as a guideline. We don't want to
> > encourage people to dis-respect line length. 
> 
> I do not recall that we (the WebKit OpenSource Project) ever agreed to
> PEP-8's maximum line length rule. I certainly do not agree to it. It is
> archaic and hurts readability. These sentiments are echoed in the thread
> that includes
> <https://lists.webkit.org/pipermail/webkit-dev/2010-April/012486.html>.

I forgot to mention that following PEP8's maximum line length rule came up in bug 184039, comment 14.
Comment 11 Aakash Jain 2018-04-03 20:41:34 PDT
> I do not recall that we (the WebKit OpenSource Project) ever agreed to
> PEP-8's maximum line length rule. I certainly do not agree to it. It is
> archaic and hurts readability. These sentiments are echoed in the thread
> that includes
> <https://lists.webkit.org/pipermail/webkit-dev/2010-April/012486.html>.

You are right, we do not follow PEP8 maximum line length rule. Our style checking script explicitly ignore this check.  https://trac.webkit.org/browser/webkit/trunk/Tools/Scripts/webkitpy/style/checker.py#L118

I felt it was ok to not document it in our Style guideline as following this rule doesn't hurt. However, if this missing documentation is creating any real problems, I would be ok if you want to document this.
Comment 12 Ryosuke Niwa 2018-04-04 16:00:19 PDT
Yeah, we certainly shouldn't adopt maximum line length.