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

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