FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

doc: add managing multiple builds section to benchmarking guide by hekike · Pull Request #16142 · nodejs/node · GitHub

/ node Public

doc: add managing multiple builds section to benchmarking guide - #16142

Closed
hekike wants to merge 1 commit into
nodejs:masterfrom
hekike:docs/guide-writing-and-running-benchmarks-multiple-builds
ConversationCommits (1)ChecksFiles changed
Closed

doc: add managing multiple builds section to benchmarking guide#16142
hekike wants to merge 1 commit into
nodejs:masterfrom
hekike:docs/guide-writing-and-running-benchmarks-multiple-builds

Conversation

hekike commented Oct 11, 2017

Copy link
Copy Markdown
Contributor
Checklist
Affected core subsystem(s)
  • doc

nodejs-github-bot added the doc Issues and PRs related to the documentations. label Oct 11, 2017
hekike mentioned this pull request Oct 17, 2017
Closed
4 tasks
Fishrock123 previously requested changes Oct 19, 2017
View reviewed changes
Comment thread doc/guides/writing-and-running-benchmarks.md Outdated

Fishrock123 Oct 19, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Spelling: parallel, also maybe put an in before parallel.

hekike force-pushed the docs/guide-writing-and-running-benchmarks-multiple-builds branch from 90e5d17 to cb9fe06 Compare October 19, 2017 18:12

hekike commented Oct 19, 2017

Copy link
Copy Markdown
Contributor Author

@Fishrock123 thanks, fixed!

apapirovski approved these changes Oct 26, 2017
View reviewed changes

apapirovski left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM — just some nits

Comment thread doc/guides/writing-and-running-benchmarks.md Outdated

apapirovski Oct 26, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Could this not be italics? I think it flows well with the rest of the copy here so it doesn't need to stand out.

Comment thread doc/guides/writing-and-running-benchmarks.md Outdated

apapirovski Oct 26, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Maybe remove the linebreak before this.

apapirovski commented Oct 26, 2017

Copy link
Copy Markdown
Contributor

@Fishrock123 Would you mind reviewing this as it appears your feedback has been addressed?

hekike force-pushed the docs/guide-writing-and-running-benchmarks-multiple-builds branch from cb9fe06 to 776ac9c Compare October 26, 2017 12:14

hekike commented Oct 26, 2017

Copy link
Copy Markdown
Contributor Author

@apapirovski thanks, fixed!

nodejs deleted a comment from nitalynn1 Oct 26, 2017

apapirovski commented Oct 26, 2017

Copy link
Copy Markdown
Contributor

CI: https://ci.nodejs.org/job/node-test-pull-request/11007/ (just for all the linting & markdown checking, will stop after that job)

apapirovski pushed a commit that referenced this pull request Oct 27, 2017
doc: add multiple build guide to benchmarking doc
2438877 
PR-URL: #16142
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

apapirovski commented Oct 27, 2017

Copy link
Copy Markdown
Contributor

Landed in 2438877

Thanks @hekike! Congrats on becoming a Contributor!!

jasnell mentioned this pull request Oct 30, 2017
Merged
gibfahn pushed a commit that referenced this pull request Oct 30, 2017
doc: add multiple build guide to benchmarking doc
5257850 
PR-URL: #16142
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
gibfahn pushed a commit that referenced this pull request Oct 30, 2017
doc: add multiple build guide to benchmarking doc
95c9a8d 
PR-URL: #16142
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
gibfahn pushed a commit that referenced this pull request Oct 31, 2017
doc: add multiple build guide to benchmarking doc
83902e6 
PR-URL: #16142
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
gibfahn mentioned this pull request Oct 31, 2017
Merged
Qard pushed a commit to ayojs/ayo that referenced this pull request Nov 2, 2017
doc: add multiple build guide to benchmarking doc
b364a58 
PR-URL: nodejs/node#16142
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Qard pushed a commit to ayojs/ayo that referenced this pull request Nov 2, 2017
doc: add multiple build guide to benchmarking doc
1605175 
PR-URL: nodejs/node#16142
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit to ayojs/ayo that referenced this pull request Dec 7, 2017
doc: add multiple build guide to benchmarking doc
0b5e9f7 
PR-URL: nodejs/node#16142
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell approved these changes
+2 more reviewers
apapirovski apapirovski approved these changes
Fishrock123 Fishrock123 left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

doc Issues and PRs related to the documentations.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

Back | FazBrowse Home | New Git URL