Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.2k
doc: make the documentation print-friendly#6748
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
sorcamarian commented May 13, 2016 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Qard commented May 13, 2016 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
LGTM, but please add |
Qard commented May 13, 2016
Sorry, I meant the commit itself, not just the PR. You'll need to do a |
MylesBorins commented May 13, 2016
/cc @nodejs/website |
sorcamarian commented May 14, 2016
@Qard I did --ammend, and --force push |
doc/api_assets/style.css Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: missing newline at the end of file.
sorcamarian commented May 14, 2016
@ChALkeR I updated the commit. |
doc/api_assets/style.css Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New line still missing here.
lpinca commented May 14, 2016
Please edit the commit message while you are at it: Make -> make. |
doc/api_assets/style.css Outdated
ChALkeRMay 14, 2016 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace
html{with
html{as I noted above (one space between html and { instead of two).
sorcamarian commented May 14, 2016
White spaces removed. |
sorcamarian commented May 16, 2016
Has something remained? |
addaleax commented May 16, 2016
@sorcamarian I think this is good to go. Like @lpinca said in his last comment, the commit message subject should be all-lowercase, but things like this can be taken of when landing the commit. If you’re asking why this isn’t merged yet, it’s just that it’s customary to wait a bit (72 hours over weekends is a general rule) before merging so that everyone has a chance to look at it. I’d suggest you additionally use Also: You author name in this commit is given as “Marian”. Is that intended or do you prefer to be listed (changelog, git log, AUTHORS file) with some other name? People typically prefer their full name, but ultimately it’s up to you. |
sorcamarian commented May 16, 2016
@addaleax Thanks for the given explanations. I'm fine with the current name("Marian"). |
MylesBorins commented May 16, 2016
LGTM... minor nits regarding newlines + commit message can be fixed when landing Would like to hear something from more folks from @nodejs/documentation and @nodejs/website |
lpinca commented May 16, 2016 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
All nits seem to be fixed. |
addaleax commented May 18, 2016
LGTM |
sorcamarian commented May 23, 2016
Any news? |
Fixes: #6743 PR-URL: #6748 Reviewed-By: Robert Jefe Lindstaedt <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Myles Borins <[email protected]>
eljefedelrodeodeljefe commented May 23, 2016
Landed in c161849. Thanks! |

Checklist
Affected core subsystem(s)
Description of change