Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2084963002:
Layout Test needs manual rebaseline due to issue v8:4955 (Closed)
DescriptionLayout Test needs manual rebaseline due to issue v8:4955
The expected output of
inspector-protocol/console/console-let-const-with-api.html needs to
change which type of error is expected from variable redeclaration.
It used to be TypeError; now it's SyntaxError.
BUG=621382
LOG=n
Committed: https://crrev.com/c932752b9e1e58cb30262b0bd213005c27c4fcde
Cr-Commit-Position: refs/heads/master@{#401067}
Patch Set 1 #
Total comments: 3
Patch Set 2 : remove comment #Messages
Total messages: 16 (7 generated)
lgtm
Description was changed from ========== Layout Test needs manual rebaseline due to issue v8:4955 The expected output of inspector-protocol/console/console-let-const-with-api.html needs to change which type of error is expected from variable redeclaration. It used to be TypeError; now it's SyntaxError. BUG=621382 R=littledan@chromium.org ========== to ========== Layout Test needs manual rebaseline due to issue v8:4955 The expected output of inspector-protocol/console/console-let-const-with-api.html needs to change which type of error is expected from variable redeclaration. It used to be TypeError; now it's SyntaxError. BUG=621382 R=littledan@chromium.org ==========
jwolfe@igalia.com changed reviewers: + adamk@chromium.org
Description was changed from ========== Layout Test needs manual rebaseline due to issue v8:4955 The expected output of inspector-protocol/console/console-let-const-with-api.html needs to change which type of error is expected from variable redeclaration. It used to be TypeError; now it's SyntaxError. BUG=621382 R=littledan@chromium.org ========== to ========== Layout Test needs manual rebaseline due to issue v8:4955 The expected output of inspector-protocol/console/console-let-const-with-api.html needs to change which type of error is expected from variable redeclaration. It used to be TypeError; now it's SyntaxError. BUG=621382 LOG=n ==========
lgtm % comment https://codereview.chromium.org/2084963002/diff/1/third_party/WebKit/LayoutTe... File third_party/WebKit/LayoutTests/TestExpectations (right): https://codereview.chromium.org/2084963002/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/TestExpectations:816: # changing test expectations Please remove this comment, it doesn't add anything, since "[ NeedsManualRebaseline ]" already implies this.
https://codereview.chromium.org/2084963002/diff/1/third_party/WebKit/LayoutTe... File third_party/WebKit/LayoutTests/TestExpectations (right): https://codereview.chromium.org/2084963002/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/TestExpectations:816: # changing test expectations On 2016/06/21 17:06:15, adamk wrote: > Please remove this comment, it doesn't add anything, since "[ > NeedsManualRebaseline ]" already implies this. Good to know. I was worried people would think the comment several lines up applied to this line. So I suppose a comment is only applicable until a blank line? or maybe no one cares this much and i'm overthinking it?
The CQ bit was checked by jwolfe@igalia.com
The patchset sent to the CQ was uploaded after l-g-t-m from littledan@chromium.org, adamk@chromium.org Link to the patchset: https://codereview.chromium.org/2084963002/#ps20001 (title: "remove comment")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2084963002/20001
https://codereview.chromium.org/2084963002/diff/1/third_party/WebKit/LayoutTe... File third_party/WebKit/LayoutTests/TestExpectations (right): https://codereview.chromium.org/2084963002/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/TestExpectations:816: # changing test expectations On 2016/06/21 17:12:51, jwolfe wrote: > On 2016/06/21 17:06:15, adamk wrote: > > Please remove this comment, it doesn't add anything, since "[ > > NeedsManualRebaseline ]" already implies this. > > Good to know. I was worried people would think the comment several lines up > applied to this line. So I suppose a comment is only applicable until a blank > line? or maybe no one cares this much and i'm overthinking it? I'd say blank lines separate "sections", yeah. There's no real style guide here, but in general the details of "why" should be in the bug.
Message was sent while issue was closed.
Description was changed from ========== Layout Test needs manual rebaseline due to issue v8:4955 The expected output of inspector-protocol/console/console-let-const-with-api.html needs to change which type of error is expected from variable redeclaration. It used to be TypeError; now it's SyntaxError. BUG=621382 LOG=n ========== to ========== Layout Test needs manual rebaseline due to issue v8:4955 The expected output of inspector-protocol/console/console-let-const-with-api.html needs to change which type of error is expected from variable redeclaration. It used to be TypeError; now it's SyntaxError. BUG=621382 LOG=n ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Layout Test needs manual rebaseline due to issue v8:4955 The expected output of inspector-protocol/console/console-let-const-with-api.html needs to change which type of error is expected from variable redeclaration. It used to be TypeError; now it's SyntaxError. BUG=621382 LOG=n ========== to ========== Layout Test needs manual rebaseline due to issue v8:4955 The expected output of inspector-protocol/console/console-let-const-with-api.html needs to change which type of error is expected from variable redeclaration. It used to be TypeError; now it's SyntaxError. BUG=621382 LOG=n Committed: https://crrev.com/c932752b9e1e58cb30262b0bd213005c27c4fcde Cr-Commit-Position: refs/heads/master@{#401067} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/c932752b9e1e58cb30262b0bd213005c27c4fcde Cr-Commit-Position: refs/heads/master@{#401067} |
