Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: added note warning about change to console.endTime() #6454

Closed
wants to merge 1 commit into from

Conversation

ben-page
Copy link
Contributor

@ben-page ben-page commented Apr 28, 2016

Checklist
  • documentation is changed or added
  • the commit message follows commit guidelines
Affected core subsystem(s)

doc

Description of change

I added a note warning about a change to console.endTime(). Prior to #3562, you could call console.endTime multiple times for the same label. This was unintended functionality, but the change needed to be documented.

Related Issue: #6452

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Apr 28, 2016
@@ -230,6 +230,11 @@ console.timeEnd('100-elements');
// prints 100-elements: 225.438ms
```

*Note: As of node v6.0.0, timeEnd() deletes the timer to avoid leaking it. On
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use Node.js v6.0.0 here please?

Maybe also instead of "timeEnd()", do console.timeEnd()

@evanlucas
Copy link
Contributor

Thanks for the PR @ben-page! I made a few comments. Let me know if you have any questions.

/cc @nodejs/documentation

@jasnell
Copy link
Member

jasnell commented Apr 28, 2016

LGTM with nits addressed

@eljefedelrodeodeljefe
Copy link
Contributor

LGTM with @evanlucas nits addressed. Thx.

Unintended functionality was removed from console.endTime by
nodejs#3562. Prior to that, you could
call console.endTime multiple times for the same label.
@ben-page
Copy link
Contributor Author

Thanks for the feedback. I made the changes.

@mscdex mscdex added the console Issues and PRs related to the console subsystem. label Apr 29, 2016
@thefourtheye
Copy link
Contributor

LGTM

1 similar comment
@whitlockjc
Copy link
Contributor

whitlockjc commented Apr 29, 2016

LGTM

jasnell pushed a commit that referenced this pull request Apr 30, 2016
Unintended functionality was removed from console.endTime by
#3562. Prior to that, you could
call console.endTime multiple times for the same label.

PR-URL: #6454
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Robert Lindstaedt <robert.lindstaedt@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Jeremy Whitlock <jwhitlock@apache.org>
@jasnell
Copy link
Member

jasnell commented Apr 30, 2016

Landed in 541965f

@jasnell jasnell closed this Apr 30, 2016
Fishrock123 pushed a commit that referenced this pull request May 4, 2016
Unintended functionality was removed from console.endTime by
#3562. Prior to that, you could
call console.endTime multiple times for the same label.

PR-URL: #6454
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Robert Lindstaedt <robert.lindstaedt@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Jeremy Whitlock <jwhitlock@apache.org>
joelostrowski pushed a commit to joelostrowski/node that referenced this pull request May 4, 2016
Unintended functionality was removed from console.endTime by
nodejs#3562. Prior to that, you could
call console.endTime multiple times for the same label.

PR-URL: nodejs#6454
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Robert Lindstaedt <robert.lindstaedt@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Jeremy Whitlock <jwhitlock@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
console Issues and PRs related to the console subsystem. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants