Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2694293003:
Simplify specifiers when all platforms are covered, and sort lines. (Closed)
DescriptionSimplify specifiers when all platforms are covered, and sort lines.
BUG=655201
Review-Url: https://codereview.chromium.org/2694293003
Cr-Commit-Position: refs/heads/master@{#451532}
Committed: https://chromium.googlesource.com/chromium/src/+/3554f65cd92e8391454b0698510b84f6864749c8
Patch Set 1 #
Total comments: 5
Patch Set 2 : Rebased #
Messages
Total messages: 19 (12 generated)
qyearsley@chromium.org changed reviewers: + jeffcarp@chromium.org
https://codereview.chromium.org/2694293003/diff/1/third_party/WebKit/Tools/Sc... File third_party/WebKit/Tools/Scripts/webkitpy/w3c/wpt_expectations_updater_unittest.py (right): https://codereview.chromium.org/2694293003/diff/1/third_party/WebKit/Tools/Sc... third_party/WebKit/Tools/Scripts/webkitpy/w3c/wpt_expectations_updater_unittest.py:170: 'crbug.com/test [ Mac10.10 ] external/fake/test/zzzz.html [ Failure ]', Another test added here to assert that added lines should be sorted by test. https://codereview.chromium.org/2694293003/diff/1/third_party/WebKit/Tools/Sc... third_party/WebKit/Tools/Scripts/webkitpy/w3c/wpt_expectations_updater_unittest.py:189: 'Linux': ['TRUSTY'], Capitalization changed to test the case-insensitivity of simplify_specifiers. https://codereview.chromium.org/2694293003/diff/1/third_party/WebKit/Tools/Sc... third_party/WebKit/Tools/Scripts/webkitpy/w3c/wpt_expectations_updater_unittest.py:196: self.assertEqual(WPTExpectationsUpdater.simplify_specifiers(['Mac', 'Win', 'Linux'], macros), []) This line tests the case where all OS types are covered.
The CQ bit was checked by qyearsley@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
qyearsley@chromium.org changed reviewers: + tkent@chromium.org
lgtm https://codereview.chromium.org/2694293003/diff/1/third_party/WebKit/Tools/Sc... File third_party/WebKit/Tools/Scripts/webkitpy/w3c/wpt_expectations_updater_unittest.py (right): https://codereview.chromium.org/2694293003/diff/1/third_party/WebKit/Tools/Sc... third_party/WebKit/Tools/Scripts/webkitpy/w3c/wpt_expectations_updater_unittest.py:154: # test expectation lines are sorted by test, and then specifier. If this comment is at the beginning of a function, should it be a docstring?
https://codereview.chromium.org/2694293003/diff/1/third_party/WebKit/Tools/Sc... File third_party/WebKit/Tools/Scripts/webkitpy/w3c/wpt_expectations_updater_unittest.py (right): https://codereview.chromium.org/2694293003/diff/1/third_party/WebKit/Tools/Sc... third_party/WebKit/Tools/Scripts/webkitpy/w3c/wpt_expectations_updater_unittest.py:154: # test expectation lines are sorted by test, and then specifier. On 2017/02/17 at 20:56:38, jeffcarp wrote: > If this comment is at the beginning of a function, should it be a docstring? I don't personally think so -- in this case (and in most cases in unit test test methods), I think that the comments don't explain what the function does; they just explain some particular detail, so they're not really like docstrings in normal functions.
The CQ bit was checked by qyearsley@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromeos_amd64-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by qyearsley@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from jeffcarp@chromium.org Link to the patchset: https://codereview.chromium.org/2694293003/#ps20001 (title: "Rebased")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1487542632947960,
"parent_rev": "e0b4961639a9d8781023e68392847f6fe37f19d9", "commit_rev":
"3554f65cd92e8391454b0698510b84f6864749c8"}
Message was sent while issue was closed.
Description was changed from ========== Simplify specifiers when all platforms are covered, and sort lines. BUG=655201 ========== to ========== Simplify specifiers when all platforms are covered, and sort lines. BUG=655201 Review-Url: https://codereview.chromium.org/2694293003 Cr-Commit-Position: refs/heads/master@{#451532} Committed: https://chromium.googlesource.com/chromium/src/+/3554f65cd92e8391454b0698510b... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/3554f65cd92e8391454b0698510b... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
