Bug 171439 - Use PEP8 style guide for raising exceptions in webkitpy.
Summary: Use PEP8 style guide for raising exceptions in webkitpy.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Aakash Jain
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-04-28 11:05 PDT by Aakash Jain
Modified: 2017-05-01 13:23 PDT (History)
9 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Aakash Jain 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.
Comment 1 Aakash Jain 2017-04-28 11:07:38 PDT
Created attachment 308559 [details]
Proposed path
Comment 2 Daniel Bates 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.
Comment 3 Aakash Jain 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.
Comment 4 WebKit Commit Bot 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>
Comment 5 WebKit Commit Bot 2017-04-29 22:02:25 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 Alexey Proskuryakov 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.
Comment 7 Alexey Proskuryakov 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