Bug 171439

Summary:

Use PEP8 style guide for raising exceptions in webkitpy.

Product:

WebKit

Reporter:

Aakash Jain <aakash_jain>

Component:

Tools / Tests

Assignee:

Aakash Jain <aakash_jain>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

aakash_jain, ap, buildbot, commit-queue, dbates, dean_johnson, dewei_zhu, glenn, lforschler

Priority:

Version:

WebKit Local Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Proposed path	dbates: review+, dbates: commit-queue-
Updated patch.	none

Aakash Jain

Reported 2017-04-28 11:05:59 PDT

As per https://www.python.org/dev/peps/pep-0008/ "When raising an exception in Python 2, use raise ValueError('message') instead of the older form raise ValueError, 'message'" in webkitpy we are still using older folrm to raise exception. We should switch to new form.

Attachments
Proposed path (8.07 KB, patch) 2017-04-28 11:07 PDT, Aakash Jain	dbates: review+ dbates: commit-queue-	Details Formatted Diff Diff
Updated patch. (8.05 KB, patch) 2017-04-29 20:46 PDT, Aakash Jain	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Aakash Jain

Comment 1 2017-04-28 11:07:38 PDT

Created attachment 308559 [details] Proposed path

Daniel Bates

Comment 2 2017-04-29 15:40:36 PDT

Comment on attachment 308559 [details] Proposed path View in context: https://bugs.webkit.org/attachment.cgi?id=308559&action=review > Tools/ChangeLog:9 > + "When raising an exception in Python 2, use raise ValueError('message') Does check-webkit-style check for this? If not, we should teach it to check for this style. > Tools/Scripts/webkitpy/common/net/irc/ircbot.py:-38 > - raise NotImplementedError, "subclasses must implement" We should also take this opportunity to switch to single quoted string literals. > Tools/Scripts/webkitpy/common/net/irc/ircbot.py:41 > + raise NotImplementedError("subclasses must implement") Ditto. > Tools/Scripts/webkitpy/common/net/irc/ircbot.py:44 > + raise NotImplementedError("subclasses must implement") Ditto. > Tools/Scripts/webkitpy/common/thread/messagepump.py:32 > + raise NotImplementedError("subclasses must implement") Ditto. > Tools/Scripts/webkitpy/common/thread/messagepump.py:35 > + raise NotImplementedError("subclasses must implement") Ditto. > Tools/Scripts/webkitpy/common/thread/messagepump.py:38 > + raise NotImplementedError("subclasses must implement") Ditto. > Tools/Scripts/webkitpy/tool/multicommandtool.py:133 > + raise NotImplementedError("subclasses must implement") Ditto. > Tools/Scripts/webkitpy/tool/multicommandtool.py:265 > + raise NotImplementedError("subclasses must implement") Ditto. > Tools/Scripts/webkitpy/tool/bot/queueengine.py:52 > + raise NotImplementedError("subclasses must implement") Ditto. > Tools/Scripts/webkitpy/tool/bot/queueengine.py:55 > + raise NotImplementedError("subclasses must implement") Ditto. > Tools/Scripts/webkitpy/tool/bot/queueengine.py:58 > + raise NotImplementedError("subclasses must implement") Ditto. > Tools/Scripts/webkitpy/tool/bot/queueengine.py:61 > + raise NotImplementedError("subclasses must implement") Ditto. > Tools/Scripts/webkitpy/tool/bot/queueengine.py:64 > + raise NotImplementedError("subclasses must implement") Ditto. > Tools/Scripts/webkitpy/tool/bot/queueengine.py:67 > + raise NotImplementedError("subclasses must implement") Ditto. > Tools/Scripts/webkitpy/tool/bot/queueengine.py:70 > + raise NotImplementedError("subclasses must implement") Ditto. > Tools/Scripts/webkitpy/tool/commands/queues.py:121 > + raise NotImplementedError("subclasses must implement") Ditto. > Tools/Scripts/webkitpy/tool/commands/queues.py:141 > + raise NotImplementedError("subclasses must implement") Ditto. > Tools/Scripts/webkitpy/tool/commands/queues.py:144 > + raise NotImplementedError("subclasses must implement") Ditto. > Tools/Scripts/webkitpy/tool/commands/queues.py:147 > + raise NotImplementedError("subclasses must implement") Ditto. > Tools/Scripts/webkitpy/tool/commands/stepsequence.py:43 > + raise NotImplementedError("subclasses must implement") Ditto. > Tools/Scripts/webkitpy/tool/commands/stepsequence.py:47 > + raise NotImplementedError("subclasses must implement") Ditto. > Tools/Scripts/webkitpy/tool/steps/abstractstep.py:79 > + raise NotImplementedError("subclasses must implement") Ditto.

Aakash Jain

Comment 3 2017-04-29 20:46:09 PDT

Created attachment 308681 [details] Updated patch. (In reply to Daniel Bates from comment #2) > Does check-webkit-style check for this? If not, we should teach it to check > for this style. Yes, it does. e.g. : "ERROR: Tools/Scripts/webkitpy/tool/bot/queueengine.py:52: deprecated form of raising exception [pep8/W602] [5]" > We should also take this opportunity to switch to single quoted string > literals. switched.

WebKit Commit Bot

Comment 4 2017-04-29 22:02:24 PDT

Comment on attachment 308681 [details] Updated patch. Clearing flags on attachment: 308681 Committed r215982: <http://trac.webkit.org/changeset/215982>

WebKit Commit Bot

Comment 5 2017-04-29 22:02:25 PDT

All reviewed patches have been landed. Closing bug.

Alexey Proskuryakov

Comment 6 2017-05-01 13:20:40 PDT

I think that the rule we used to have was to give WebKit coding style a higher priority than PEP8.

Alexey Proskuryakov

Comment 7 2017-05-01 13:23:15 PDT

But can't really find a record of that. The latest discussion of PEP8 was here, it seems: https://lists.webkit.org/pipermail/webkit-dev/2010-April/012435.html

Note You need to log in before you can comment on or make changes to this bug.