Chromium Code Reviews
|
|
Chromium Code Reviews|
Created:
4 years, 1 month ago by jgravelle Modified:
4 years, 1 month ago Reviewers:
Derek Schuff, ahaas, Yang CC:
Derek Schuff, titzer, v8-reviews_googlegroups.com Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
DescriptionAdd Shell::PrintErr and expose it in the d8 shell as printErr
This function is implemented in other JavaScript shells
BUG=None
R=titzer
Committed: https://crrev.com/fb96618ee01b70e88b997a60194548bfcc2f26e8
Cr-Commit-Position: refs/heads/master@{#40677}
Patch Set 1 #Patch Set 2 : Add simple test for print/printErr #
Total comments: 2
Patch Set 3 : Normalize and unify Print/PrintErr #Messages
Total messages: 24 (15 generated)
The CQ bit was checked by jgravelle@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_presubmit on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/27530)
The CQ bit was checked by jgravelle@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.
jgravelle@chromium.org changed reviewers: + ahaas@chromium.org - titzer@chromium.org
Not sure if the test I added is reasonable/sufficient, but I'm not sure what the best way to test printing to stderr from JS would be. Any suggestions?
dschuff@chromium.org changed reviewers: + dschuff@chromium.org
https://codereview.chromium.org/2458963003/diff/20001/src/d8.cc File src/d8.cc (right): https://codereview.chromium.org/2458963003/diff/20001/src/d8.cc#newcode967 src/d8.cc:967: Write(args); why not just `WriteToFile(stdout, args)`? Is `Write` used anywhere else?
https://codereview.chromium.org/2458963003/diff/20001/src/d8.cc File src/d8.cc (right): https://codereview.chromium.org/2458963003/diff/20001/src/d8.cc#newcode967 src/d8.cc:967: Write(args); On 2016/10/28 22:04:39, Derek Schuff wrote: > why not just `WriteToFile(stdout, args)`? Is `Write` used anywhere else? Write gets exposed to JS so most likely yes. There's more symmetry here as WriteToFile(stdout, args) though. I was erring on the side of "change as little as possible" but that's cleaner yeah.
ahaas@chromium.org changed reviewers: + yangguo@chromium.org
On 2016/10/31 at 12:11:20, ahaas wrote: > ahaas@chromium.org changed reviewers: > + yangguo@chromium.org Yang, could you please take a look?
On 2016/10/31 12:11:42, ahaas wrote: > On 2016/10/31 at 12:11:20, ahaas wrote: > > mailto:ahaas@chromium.org changed reviewers: > > + mailto:yangguo@chromium.org > > Yang, could you please take a look? lgtm.
The CQ bit was checked by jgravelle@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 jgravelle@chromium.org
The CQ bit was checked by jgravelle@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Add Shell::PrintErr and expose it in the d8 shell as printErr This function is implemented in other JavaScript shells BUG=None R=titzer ========== to ========== Add Shell::PrintErr and expose it in the d8 shell as printErr This function is implemented in other JavaScript shells BUG=None R=titzer Committed: https://crrev.com/fb96618ee01b70e88b997a60194548bfcc2f26e8 Cr-Commit-Position: refs/heads/master@{#40677} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/fb96618ee01b70e88b997a60194548bfcc2f26e8 Cr-Commit-Position: refs/heads/master@{#40677} |
