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: add "building node with ninja" guide #4767

Merged
merged 1 commit into from
Feb 23, 2016

Conversation

Fishrock123
Copy link
Contributor

People always ask me about how to do this, and there's no good info about it really anywhere.

Moving from nodejs/docs#38

@Fishrock123 Fishrock123 added the doc Issues and PRs related to the documentations. label Jan 19, 2016
@Fishrock123
Copy link
Contributor Author

cc @nodejs/documentation

@mscdex mscdex added the build Issues and PRs related to build files or the CI. label Jan 19, 2016
@chrisdickinson
Copy link
Contributor

Thanks for PR'ing this in! The Docs WG is going to discuss next steps on adding guides in tomorrows meeting at 10AM PST; we'll take action on the PR shortly after that.


The purpose of this guide is to show how to build Node.js using [Ninja][], as doing so can be much quicker than using `make`. Please see [Ninja's site][Ninja] for installation instructions (unix only).

To build Node with ninja, there are 4 steps that must be taken:
Copy link
Member

Choose a reason for hiding this comment

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

Throw in a newline after this so markdown knows it's a list. Same on line 11.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, my preview works fine, maybe it's just github. Oh well.

@bengl
Copy link
Member

bengl commented Jan 20, 2016

Please word-wrap at 80 characters, as per https://github.com/nodejs/docs/blob/master/STYLE-GUIDE.md

@Fishrock123
Copy link
Contributor Author

@bengl 80 characters does not work for lists. The list will force a newline. :/

@chrisdickinson
Copy link
Contributor

@Fishrock123 I don't think that's so — for example:

  • Hello there.
    I am wrapped.
  • I am wrapped too
    But it's okay.

Source:

* Hello there.
I am wrapped.
* I am wrapped too
But it's okay.

Edit, if that's too ugly:

  • Hello there.
    I am wrapped.
  • I am wrapped too
    But it's okay.
* Hello there.
  I am wrapped.
* I am wrapped too
  But it's okay.

Will work too!

@Fishrock123
Copy link
Contributor Author

@chrisdickinson I don't think this is necessarily consistent between parsers/renderers then:

screen shot 2016-01-22 at 3 39 06 pm

(Atom's markdown preview)

@Fishrock123
Copy link
Contributor Author

@bengl I consolidated the instructions into one list. :)

@jbergstroem
Copy link
Member

@Fishrock123 you mention a few times how much faster it is -- out of curiosity, could you quantify it?

@Fishrock123
Copy link
Contributor Author

@jbergstroem you mean /usr/bin/time it or what it seems to be?

I'd say it's at least 40% faster

@jbergstroem
Copy link
Member

@Fishrock123 so ninja vs make -j${cores}? Didn't think it was that much faster. Wow.

@Fishrock123
Copy link
Contributor Author

@jbergstroem well, that's how much faster it feels so maybe it isn't idk.

@jbergstroem
Copy link
Member

Just tested it locally:

  • time spent: ninja was 10% faster
  • cpu time: 4% less spent

@Fishrock123
Copy link
Contributor Author

ping @bengl & @chrisdickinson

@bengl
Copy link
Member

bengl commented Jan 30, 2016

ok LGTM 👍

@Fishrock123
Copy link
Contributor Author

@bengl Updated a bit (I wasn't comfortable with some statements), PTAL.

@chrisdickinson I still haven't shortened the lines... do we need to take it back to the docs WG? My editor renderer does not render split lines very nicely.

@Fishrock123
Copy link
Contributor Author

I'm going to land this tomorrow (Tuesday) unless there is additional review. We can fix any problems with it after I guess.

@bengl
Copy link
Member

bengl commented Feb 22, 2016

Whoops, missed this. Yep, LGTM.

@Fishrock123
Copy link
Contributor Author

@chrisdickinson do doc wg members officially have sign-off rights for doc/? I forget..

@Qard
Copy link
Member

Qard commented Feb 22, 2016

LGTM too. 👍

PR-URL: nodejs#4767
Refs: nodejs/docs#38
Reviewed-By: Bryan English <[email protected]>
Reviewed-By: Stephan Belanger <[email protected]>
@Fishrock123 Fishrock123 merged commit 65c0feb into nodejs:master Feb 23, 2016
rvagg pushed a commit that referenced this pull request Feb 27, 2016
PR-URL: #4767
Refs: nodejs/docs#38
Reviewed-By: Bryan English <[email protected]>
Reviewed-By: Stephan Belanger <[email protected]>
rvagg pushed a commit that referenced this pull request Feb 27, 2016
PR-URL: #4767
Refs: nodejs/docs#38
Reviewed-By: Bryan English <[email protected]>
Reviewed-By: Stephan Belanger <[email protected]>
@Fishrock123 Fishrock123 mentioned this pull request Mar 1, 2016
5 tasks
MylesBorins pushed a commit that referenced this pull request Mar 10, 2016
PR-URL: #4767
Refs: nodejs/docs#38
Reviewed-By: Bryan English <[email protected]>
Reviewed-By: Stephan Belanger <[email protected]>
MylesBorins pushed a commit that referenced this pull request Mar 17, 2016
PR-URL: #4767
Refs: nodejs/docs#38
Reviewed-By: Bryan English <[email protected]>
Reviewed-By: Stephan Belanger <[email protected]>
MylesBorins pushed a commit that referenced this pull request Mar 21, 2016
PR-URL: #4767
Refs: nodejs/docs#38
Reviewed-By: Bryan English <[email protected]>
Reviewed-By: Stephan Belanger <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants