| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
LGTM, but please add doc: to the start of the commit message. |
Sorry, something went wrong.
|
Sorry, I meant the commit itself, not just the PR. You'll need to do a git commit --amend to edit the commit message, then force push the modified commit. |
Sorry, something went wrong.
|
/cc @nodejs/website |
Sorry, something went wrong.
|
@Qard I did --ammend, and --force push |
Sorry, something went wrong.
There was a problem hiding this comment.
Nit: missing newline at the end of file.
Sorry, something went wrong.
There was a problem hiding this comment.
New line still missing here.
Sorry, something went wrong.
|
Please edit the commit message while you are at it: Make -> make. |
Sorry, something went wrong.
There was a problem hiding this comment.
Replace
html {with
html {as I noted above (one space between html and { instead of two).
Sorry, something went wrong.
|
White spaces removed. |
Sorry, something went wrong.
|
Has something remained? |
Sorry, something went wrong.
|
@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 Fixes: https://github.com/nodejs/node/issues/6743 instead of the more info line, because then the issue will be closed automatically when your commit is landed. 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. |
Sorry, something went wrong.
|
@addaleax Thanks for the given explanations. I'm fine with the current name("Marian"). |
Sorry, something went wrong.
|
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 |
Sorry, something went wrong.
|
All nits seem to be fixed. |
Sorry, something went wrong.
|
LGTM |
Sorry, something went wrong.
|
Any news? |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Checklist
Affected core subsystem(s)
Description of change