Skip to content

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

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

cc @nodejs/documentation

@mscdexmscdex 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.

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
ContributorAuthor

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

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

@Fishrock123
Copy link
ContributorAuthor

@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
ContributorAuthor

@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
ContributorAuthor

@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
ContributorAuthor

@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
ContributorAuthor

@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
ContributorAuthor

ping @bengl & @chrisdickinson

@bengl
Copy link
Member

ok LGTM 👍

@Fishrock123
Copy link
ContributorAuthor

@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
ContributorAuthor

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

Whoops, missed this. Yep, LGTM.

@Fishrock123
Copy link
ContributorAuthor

@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]>
@Fishrock123Fishrock123 merged commit 65c0feb into nodejs:masterFeb 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]>
@Fishrock123Fishrock123 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 freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

buildIssues and PRs related to build files or the CI.docIssues and PRs related to the documentations.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants

@Fishrock123@chrisdickinson@bengl@jbergstroem@Qard@sam-github@kenany@gibfahn@mscdex@MylesBorins