console: implement timeLog method by targos · Pull Request #21312 · nodejs/node

targos · 2018-06-13T13:41:06Z

Marking blocked until the spec PR lands.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

nodejs-github-bot · 2018-06-13T13:41:07Z

@targos build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/50/pipeline

cjihrig · 2018-06-13T14:11:37Z

lib/console.js

Other than the function name printed in the process warning, couldn't Console.prototype.timeEnd() be updated to call this?

apapirovski

Barring spec changes, this looks great.

vsemozhetbyt · 2018-06-13T20:10:09Z

doc/api/console.md

Extra parenthesis in [`console.time()`)][].

vsemozhetbyt · 2018-06-13T20:11:33Z

doc/api/console.md

Maybe add a comment that value is 42 here? Otherwise, it is a bit unexpected below.

vsemozhetbyt · 2018-06-13T20:13:25Z

doc/api/console.md

Seems a bit confusing. Maybe something like // Prints "process: 365.227ms 42".?

vsemozhetbyt · 2018-06-13T20:15:50Z

Should we document the separator that joins several `...data` values?

vsemozhetbyt · 2018-06-13T20:20:39Z

doc/api/console.md

It seems this should go after ### console.timeEnd(label) section ABC-wise.

No strong opinion but this document doesn't seem to follow the ABC rule and I went for the spec order. It's also the order in which you call the methods

All console. methods seem to be in ABC order according to the TOC.

Right. I didn't see that inspector methods were in another section

targos · 2018-06-16T17:14:35Z

@cjihrig I refactored the common code in a separate function. PTAL.

@vsemozhetbyt

Should we document the separator that joins several ...data values?

I don't know. It's just using console.log()'s mechanics. Perhaps we should document that? I don't know how to write it though.

vsemozhetbyt · 2018-06-16T17:48:17Z

@targos Then we can land as is and if anybody is sure how to document this (if needed), this can be done in another PR)

BridgeAR · 2018-06-18T11:11:47Z

doc/api/console.md

This will print a warning about no process label, right?

It's not supposed to. timeLog shouldn't clear the timer.

BridgeAR · 2018-06-18T11:18:36Z

lib/console.js

Nit: I personally would prefer to just check if data is undefined by adding a if / else but as the function is not hot, that is just a non-blocking recommendation.

I can do that 👍

apapirovski · 2018-06-25T07:00:19Z

CI: https://ci.nodejs.org/job/node-test-pull-request/15599/

jasnell · 2018-06-29T20:39:44Z

Looks like this failed across the board.

Refs: whatwg/console#138

targos · 2018-06-30T12:59:04Z

CI: https://ci.nodejs.org/job/node-test-pull-request/15686/

targos · 2018-07-04T06:26:24Z

Upstream PR landed: https://console.spec.whatwg.org/#timelog

targos · 2018-07-04T19:16:22Z

CI before landing: https://ci.nodejs.org/job/node-test-pull-request/15728/

targos · 2018-07-04T20:14:51Z

Landed in b016745

Refs: whatwg/console#138 PR-URL: #21312 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Weijia Wang <starkwang@126.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com>

Notable changes: * console: * The `console.timeLog()` method has been implemented. (#21312) * deps: * Upgrade to libuv 1.22.0. (#21731) * Upgrade to ICU 62.1 (Unicode 11, CLDR 33.1). (#21728) * http: * Added support for passing both `timeout` and `agent` options to `http.request`. (#21204) * napi: * Added experimental support for functions dealing with bigint numbers. (#21226) * process: * The `process.hrtime.bigint()` method has been implemented. (#21256) * Added the `--title` command line argument to set the process title on startup. (#21477) * trace_events: * Added process_name metadata. (#21477)

Notable changes: * console: * The `console.timeLog()` method has been implemented. (#21312) * deps: * Upgrade to libuv 1.22.0. (#21731) * Upgrade to ICU 62.1 (Unicode 11, CLDR 33.1). (#21728) * http: * Added support for passing both `timeout` and `agent` options to `http.request`. (#21204) * napi: * Added experimental support for functions dealing with bigint numbers. (#21226) * process: * The `process.hrtime.bigint()` method has been implemented. (#21256) * Added the `--title` command line argument to set the process title on startup. (#21477) * trace_events: * Added process_name metadata. (#21477) * Added new collaborators * codebytere - Shelley Vohr PR-URL: #21851

Notable changes: * console: * The `console.timeLog()` method has been implemented. (#21312) * deps: * Upgrade to libuv 1.22.0. (#21731) * Upgrade to ICU 62.1 (Unicode 11, CLDR 33.1). (#21728) * http: * Added support for passing both `timeout` and `agent` options to `http.request`. (#21204) * inspector: * Expose the original console API in `require('inspector').console`. (#21659) * napi: * Added experimental support for functions dealing with bigint numbers. (#21226) * process: * The `process.hrtime.bigint()` method has been implemented. (#21256) * Added the `--title` command line argument to set the process title on startup. (#21477) * trace_events: * Added process_name metadata. (#21477) * Added new collaborators * codebytere - Shelley Vohr PR-URL: #21851

targos added wip Issues and PRs that are still a work in progress. semver-minor PRs that contain new features and should be released in the next minor version. console Issues and PRs related to the console subsystem. labels Jun 13, 2018

nodejs-github-bot added the console Issues and PRs related to the console subsystem. label Jun 13, 2018

targos added blocked PRs that are blocked by other issues or PRs. and removed wip Issues and PRs that are still a work in progress. labels Jun 13, 2018

cjihrig approved these changes Jun 13, 2018

View reviewed changes

devsnek approved these changes Jun 13, 2018

View reviewed changes

apapirovski approved these changes Jun 13, 2018

View reviewed changes

vsemozhetbyt reviewed Jun 13, 2018

View reviewed changes

starkwang approved these changes Jun 14, 2018

View reviewed changes

targos force-pushed the console-timelog branch from e6f1714 to 73c32ca Compare June 16, 2018 16:52

BridgeAR approved these changes Jun 18, 2018

View reviewed changes

jasnell approved these changes Jun 21, 2018

View reviewed changes

targos force-pushed the console-timelog branch from 63bd821 to 67279d0 Compare June 30, 2018 12:41

console: implement timeLog method

32bae6b

Refs: whatwg/console#138

targos force-pushed the console-timelog branch from 67279d0 to 32bae6b Compare June 30, 2018 12:50

domfarolino mentioned this pull request Jul 4, 2018

console: console.countReset() should emit warning #21649

Closed

4 tasks

targos removed the blocked PRs that are blocked by other issues or PRs. label Jul 4, 2018

targos closed this Jul 4, 2018

targos deleted the console-timelog branch July 4, 2018 20:14

targos mentioned this pull request Jul 17, 2018

v10.7.0 proposal #21851

Merged

Uh oh!

Conversation

targos commented Jun 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

nodejs-github-bot commented Jun 13, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

apapirovski left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vsemozhetbyt commented Jun 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

targos commented Jun 16, 2018

Uh oh!

vsemozhetbyt commented Jun 16, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

targos Jun 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

apapirovski commented Jun 25, 2018

Uh oh!

jasnell commented Jun 29, 2018

Uh oh!

targos commented Jun 30, 2018

Uh oh!

targos commented Jul 4, 2018

Uh oh!

targos commented Jul 4, 2018

Uh oh!

targos commented Jul 4, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

targos commented Jun 13, 2018 •

edited

Loading

vsemozhetbyt commented Jun 13, 2018 •

edited

Loading

targos Jun 18, 2018 •

edited

Loading