Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2695713014:
[api] Use CHECK instead of DCHECK for IsModule tests in ScriptCompiler (Closed)
Description[api] Use CHECK instead of DCHECK for IsModule tests in ScriptCompiler
This is such a mis-use of the API that it's important to fail fast:
this patch was prompted by a bug report from a Node.js developer
trying to use CompileModule() without passing a properly
module-tagged ScriptOrigin.
R=jochen@chromium.org, neis@chromium.org
BUG=v8:1569
Review-Url: https://codereview.chromium.org/2695713014
Cr-Commit-Position: refs/heads/master@{#43354}
Committed: https://chromium.googlesource.com/v8/v8/+/abb9eda40d2500238432ed2c64c4491ffbaa9263
Patch Set 1 #Patch Set 2 : Use ApiCheck #Patch Set 3 : Fix build #Messages
Total messages: 27 (18 generated)
The CQ bit was checked by adamk@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...
On 2017/02/16 20:36:41, adamk wrote: yay!
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by neis@chromium.org
lgtm
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 neis@chromium.org
what about making this an ApiCheck so you can also print something meaningfully to help embedders?
Good idea, switched to ApiCheck.
The CQ bit was checked by adamk@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: Try jobs failed on following builders: v8_linux64_asan_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng/buil...)
The CQ bit was checked by adamk@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.
lgtm
The CQ bit was checked by adamk@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from neis@chromium.org Link to the patchset: https://codereview.chromium.org/2695713014/#ps40001 (title: "Fix build")
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": 40001, "attempt_start_ts": 1487699929083040,
"parent_rev": "e6819ee286c2041f9a668346f519e6fb7db9708f", "commit_rev":
"abb9eda40d2500238432ed2c64c4491ffbaa9263"}
Message was sent while issue was closed.
Description was changed from ========== [api] Use CHECK instead of DCHECK for IsModule tests in ScriptCompiler This is such a mis-use of the API that it's important to fail fast: this patch was prompted by a bug report from a Node.js developer trying to use CompileModule() without passing a properly module-tagged ScriptOrigin. R=jochen@chromium.org, neis@chromium.org BUG=v8:1569 ========== to ========== [api] Use CHECK instead of DCHECK for IsModule tests in ScriptCompiler This is such a mis-use of the API that it's important to fail fast: this patch was prompted by a bug report from a Node.js developer trying to use CompileModule() without passing a properly module-tagged ScriptOrigin. R=jochen@chromium.org, neis@chromium.org BUG=v8:1569 Review-Url: https://codereview.chromium.org/2695713014 Cr-Commit-Position: refs/heads/master@{#43354} Committed: https://chromium.googlesource.com/v8/v8/+/abb9eda40d2500238432ed2c64c4491ffba... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/v8/v8/+/abb9eda40d2500238432ed2c64c4491ffba... |
