Summary: | [webkitpy] Add ignore_errors keyword argument to Executive.run_command() for replacing the common pattern of error_handler usage | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Basuke Suzuki <Basuke.Suzuki> | ||||
Component: | Tools / Tests | Assignee: | Basuke Suzuki <Basuke.Suzuki> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | achristensen, Basuke.Suzuki, ddkilzer, ews-watchlist, glenn, lforschler, pvollan, rniwa, webkit-bug-importer | ||||
Priority: | P2 | ||||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Basuke Suzuki
2017-12-14 11:15:25 PST
Also there's no need to import Executive module just to pass Executive.ignore_error static method if this option is available. Created attachment 329373 [details]
patch
Comment on attachment 329373 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=329373&action=review r=me with a couple comments. > Tools/Scripts/webkitpy/common/system/executive.py:407 > + if ignore_errors: > + assert error_handler is None, "don't specify error_handler if ignore_errors" Nit: I would add " is True" to the end of the message to make it clear: assert error_handler is None, "don't specify error_handler if ignore_errors is True" I would prefer this to be fixed before landing unless you feel strongly about not adding " is True". > Tools/Scripts/webkitpy/common/system/executive_mock.py:198 > + if ignore_errors: > + assert error_handler is None, "don't specify error_handler if ignore_errors" Nit: I would add " is True" to the end of the message to make it clear: assert error_handler is None, "don't specify error_handler if ignore_errors is True" I would prefer this to be fixed before landing unless you feel strongly about not adding " is True". Committed r227427: <https://trac.webkit.org/changeset/227427> |