doc: update trace_event doc in tracing.md #20092

BeniCheni · 2018-04-17T05:07:18Z

Change title from tracing to trace_event in the documentation content. Update the description of node , node.async_hooks and v8 category to contain information highlighted in #16315.

Fixes: #16315

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
documentation is changed or added
commit message follows commit guidelines

P.S.: After reading #16315, the the async_hooks and v8 documentations in details, I'm hoping to give this "good first issue" issue an attempt.

Thanks for your time to review in advance. Hoping the PR looks okay. Happy to update according to any review feedback. ✌️

Trott · 2018-04-17T05:29:26Z

@nodejs/documentation @nodejs/diagnostics

vsemozhetbyt

Thank you for the effort to make docs better!
I've left some comments about format nits)

vsemozhetbyt · 2018-04-17T08:52:04Z

doc/api/tracing.md

In docs, lines are usually wrapped within 80 characters, otherwise, there will be doc linting issue.

@vsemozhetbyt, 🙏 for your advise. Learned make lint script and and updated according to linting.

vsemozhetbyt · 2018-04-17T08:55:01Z

doc/api/tracing.md

If not in code fragment, the uppercased variant is preferred: [V8]

@vsemozhetbyt, after learning make lint script, updated to the uppercased variant, ✌️

vsemozhetbyt · 2018-04-17T08:56:14Z

doc/api/tracing.md

is -> Is for consistency with other items?

I would just go with "An empty placeholder." maybe or something. Unless we have some rule about needing complete sentences...

@apapirovski & @vsemozhetbyt, 👍& 🙏for both of your suggestions. Updated to go with "An empty placeholder.", as it seems "short and sweet" for now. Happy to do follow-up PR, if there's a new content for node.

vsemozhetbyt · 2018-04-17T08:58:44Z

doc/api/tracing.md

Trace Event -> Trace Events?

Yes, events are plurals. Updated. 😅👶

vsemozhetbyt · 2018-04-17T09:03:49Z

doc/api/tracing.md

As .md files are only sources for .html documents in https://nodejs.org/api/, links usually refer to HTML docs. Also, ASCII sorting in bottom references is preferred. So this fragment may be written in such way:

[Performance API]: perf_hooks.html [V8]: v8.html [async_hooks]: async_hooks.html

@vsemozhetbyt, learned the documentation scripts (i.e. make doc) & the built .html files' relationship to the docs, the interesting ASCII sorting preferences. Will keep that in mind going forward. 🙏for this piece of valuable review comment.

BeniCheni · 2018-04-18T05:39:29Z

@vsemozhetbyt, really appreciate your time in the previous comments about format nits. I've updated, and tried my best to reply the each outdated comment accordingly, to "kind of" recap the educational takeaway from the review comments. 👨‍🎓Please have another quick review, when you have time. 👍

P.S.: I'm actually a mentee in the mentorship, awaiting to be paired with a mentor. So I just wanted to find a "good first time" labelled issue to try to contribute a "small" update, and learn the node.js ecosystem before the mentorship kicks in full gear. So really appreciate your patience about the format nits. 🙏

vsemozhetbyt

Doc format LGTM)

BeniCheni · 2018-04-19T06:59:20Z

@vsemozhetbyt , confirmed that my local master did not have the sync with upstream master. Once the local branches are sync'ed with upstream, the PR looked ok. Can you try the CI-lite build again? Thanks.

BeniCheni · 2018-04-22T06:20:39Z

Hi, thanks for re-running CI-lite: https://ci.nodejs.org/job/node-test-pull-request-lite/549/.

Just a simple question to ensure I didn't get confused. Per previous note below, is there anything I can do to help make the CI-Lite passing and the lander successful? I'm happy to help out, if there's any thing I can do to help. Thank you!

"Memo: It seems the lander may need to eliminate 2 merge commits on landing)", is there anything I can help to make the lander pass CI-lite to land? ~~(Just wanted to~~

AyushG3112 · 2018-04-22T07:16:15Z

@BeniCheni if you could rebase your branch to the upstream master, as shown here, that would help.

vsemozhetbyt · 2018-04-22T12:30:50Z

@BeniCheni It seems the rebase process is rather complicated in this case. I have tried to do this cloning your branch. If it is OK, I can push the result back and we will see if everything is OK. If something goes awry, you can just create a new PR with the same changes.

Refs: #16315

BeniCheni · 2018-04-22T15:39:23Z

~~@ Trott~~,
@vsemozhetbyt, your plan sounds great. Thanks for your time. Please push the result back, and I’ll:

look out for the “push” for the result back
close this PR
create a new PR for the same changes”.

AyushG3112 · 2018-04-22T15:47:02Z

@BeniCheni I think you might have @-tagged the wrong person

BeniCheni · 2018-04-22T16:04:04Z

Corrected the previously wrong @ -tagged person.

vsemozhetbyt · 2018-04-22T16:20:56Z

@BeniCheni I've squashed all the commits in one and force-pushed in your branch. It seems everything is OK, but please, check if the changes are identical to yours intended, If they are, we may need another positive review for this and then land as is.

vsemozhetbyt · 2018-04-22T16:25:18Z

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

vsemozhetbyt · 2018-04-22T16:26:14Z

@trivikr Please, take a look if this is still good for you.

BeniCheni · 2018-04-22T16:30:00Z

@vsemozhetbyt, I’ll reviewed and confirmed the changes are identical to mine intended.

vsemozhetbyt · 2018-04-22T16:35:15Z

CI-lite is green 🎉, so let us wait for one more positive review and then land)

trivikr · 2018-04-23T02:59:59Z

@vsemozhetbyt Looks good! 👍

PR-URL: #20092 Fixes: #16315 Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

vsemozhetbyt · 2018-04-23T06:31:33Z

Landed in 8b012e1
Thank you, @BeniCheni!

PR-URL: #20092 Fixes: #16315 Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

nodejs-github-bot added the doc Issues and PRs related to the documentations. label Apr 17, 2018

BeniCheni changed the title ~~doc: rename trace_event doc and update update content per #16315~~ doc: update trace_event doc in tracing.md Apr 17, 2018

vsemozhetbyt added the trace_events Issues and PRs related to V8, Node.js core, and userspace code trace events. label Apr 17, 2018

This comment has been minimized.

Sign in to view

vsemozhetbyt reviewed Apr 17, 2018

View reviewed changes

vsemozhetbyt approved these changes Apr 18, 2018

View reviewed changes

This comment has been minimized.

Sign in to view

trivikr approved these changes Apr 18, 2018

View reviewed changes

This comment has been minimized.

Sign in to view

doc: update trace events categories description

4a0545c

Refs: #16315

trivikr approved these changes Apr 23, 2018

View reviewed changes

vsemozhetbyt pushed a commit that referenced this pull request Apr 23, 2018

doc: update trace events categories description

8b012e1

PR-URL: #20092 Fixes: #16315 Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

vsemozhetbyt closed this Apr 23, 2018

jasnell pushed a commit that referenced this pull request Apr 23, 2018

doc: update trace events categories description

b7d1e19

PR-URL: #20092 Fixes: #16315 Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

BeniCheni mentioned this pull request Jun 18, 2018

dns.js not properly iterating through DNS array #21391

Closed

Uh oh!

doc: update trace_event doc in tracing.md #20092

doc: update trace_event doc in tracing.md #20092

Uh oh!

Conversation

BeniCheni commented Apr 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

Trott commented Apr 17, 2018

Uh oh!

This comment has been minimized.

vsemozhetbyt left a comment

Choose a reason for hiding this comment

Uh oh!

vsemozhetbyt Apr 17, 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!

BeniCheni Apr 18, 2018

Choose a reason for hiding this comment

Uh oh!

vsemozhetbyt Apr 17, 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!

BeniCheni Apr 18, 2018

Choose a reason for hiding this comment

Uh oh!

vsemozhetbyt Apr 17, 2018

Choose a reason for hiding this comment

Uh oh!

apapirovski Apr 17, 2018

Choose a reason for hiding this comment

Uh oh!

BeniCheni Apr 18, 2018

Choose a reason for hiding this comment

Uh oh!

vsemozhetbyt Apr 17, 2018

Choose a reason for hiding this comment

Uh oh!

BeniCheni Apr 18, 2018

Choose a reason for hiding this comment

Uh oh!

vsemozhetbyt Apr 17, 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!

BeniCheni Apr 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!

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

Uh oh!

Uh oh!

vsemozhetbyt left a comment

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

BeniCheni commented Apr 19, 2018

Uh oh!

This comment has been minimized.

This comment has been minimized.

BeniCheni commented Apr 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AyushG3112 commented Apr 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vsemozhetbyt commented Apr 22, 2018

Uh oh!

BeniCheni commented Apr 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AyushG3112 commented Apr 22, 2018

Uh oh!

BeniCheni commented Apr 22, 2018

Uh oh!

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

BeniCheni commented Apr 17, 2018 •

edited

Loading

vsemozhetbyt Apr 17, 2018 •

edited

Loading

vsemozhetbyt Apr 17, 2018 •

edited

Loading

vsemozhetbyt Apr 17, 2018 •

edited

Loading

BeniCheni Apr 18, 2018 •

edited

Loading

BeniCheni commented Apr 18, 2018 •

edited

Loading

BeniCheni commented Apr 22, 2018 •

edited

Loading

AyushG3112 commented Apr 22, 2018 •

edited

Loading

BeniCheni commented Apr 22, 2018 •

edited

Loading

vsemozhetbyt commented Apr 22, 2018 •

edited

Loading