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

doc: ./node to node in debugger.md by AnnaMag · Pull Request #8943 · nodejs/node · GitHub

/ node Public

doc: ./node to node in debugger.md - #8943

Closed
AnnaMag wants to merge 1 commit into
nodejs:masterfrom
AnnaMag:amk
Closed

doc: ./node to node in debugger.md#8943
AnnaMag wants to merge 1 commit into
nodejs:masterfrom
AnnaMag:amk

Conversation

AnnaMag commented Oct 5, 2016

Copy link
Copy Markdown
Member

doc: typo in line 123 of doc/api/debugger.md

example node calls use node, not ./node

Fixes: Examples in debugger documentation should all use global node #8942

nodejs-github-bot added debugger doc Issues and PRs related to the documentations. labels Oct 5, 2016

Copy link
Copy Markdown
Contributor

@AnnaMag could you prefix your commit message with doc:? thanks!

addaleax left a comment

Copy link
Copy Markdown
Member

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

Looks good!

Generally +1 to @Fishrock123’s request (you should be able to edit the commit message using git commit --amend && git push -f, just in case). That can also be applied when merging the commit, so don’t worry about that.

AnnaMag commented Oct 5, 2016
edited
Loading

Copy link
Copy Markdown
Member Author

@addaleax, @Fishrock123 Ok! next time will keep that convention in mind.

lpinca left a comment

Copy link
Copy Markdown
Member

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

fhinkel commented Oct 6, 2016

Copy link
Copy Markdown
Contributor

Thanks! Can you prefix the commit message with doc: please?

jasnell commented Oct 6, 2016

Copy link
Copy Markdown
Member

LGTM. I think this is trivial enough that it does not have to wait the full 48 hours to land.

addaleax commented Oct 6, 2016

Copy link
Copy Markdown
Member

@AnnaMag Hi, not sure where the second commit here is coming from, but that seems like something that should go into a different pull request?

(If you have any questions, always feel free to ask, here or e.g. in #node-dev on Freenode!)

AnnaMag commented Oct 6, 2016

Copy link
Copy Markdown
Member Author

@addaleax upps! I added a new commit to the same branch on my remote.
It automatically updated this pull request with a new commit, which I had no idea it would do, as I did not do a pull request yet.
Should closing this pull request solve the problem or should I move the commit to a different branch?

lpinca commented Oct 6, 2016

Copy link
Copy Markdown
Member

@AnnaMag you can remove the wrong commit and force push, no need to close the pr.

AnnaMag commented Oct 6, 2016
edited
Loading

Copy link
Copy Markdown
Member Author

done! apologies for the confusion:)

addaleax commented Oct 6, 2016

Copy link
Copy Markdown
Member

No problem, I was just surprised.

I agree with @jasnell, this doesn’t need to wait 48 hours, so I’ll go ahead and merge this PR. :)

addaleax self-assigned this Oct 6, 2016

addaleax commented Oct 6, 2016

Copy link
Copy Markdown
Member

Oh, by the way – your author name in this commit is given as AnnaMag <AnnaMag@users.noreply.github.com>. 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.

AnnaMag commented Oct 6, 2016

Copy link
Copy Markdown
Member Author

@addaleax, I appreciate you pointing it out. I am fine with the way it is at the moment. Thanks though!

addaleax pushed a commit that referenced this pull request Oct 6, 2016
Fixes: #8942
PR-URL: #8943
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

addaleax commented Oct 6, 2016

Copy link
Copy Markdown
Member

Aye aye then!

Landed this in bf0bcf4, thanks for the contribution!

addaleax closed this Oct 6, 2016
jasnell pushed a commit that referenced this pull request Oct 10, 2016
Fixes: #8942
PR-URL: #8943
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Fishrock123 pushed a commit that referenced this pull request Oct 11, 2016
Fixes: #8942
PR-URL: #8943
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins pushed a commit that referenced this pull request Nov 18, 2016
Fixes: #8942
PR-URL: #8943
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins mentioned this pull request Nov 22, 2016
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc Issues and PRs related to the documentations.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants


Back | FazBrowse Home | New Git URL