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: ./node to node in debugger.md #8943

Closed
wants to merge 1 commit into from
Closed

Conversation

AnnaMag
Copy link
Member

@AnnaMag AnnaMag commented Oct 5, 2016

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 nodejs-github-bot added debugger doc Issues and PRs related to the documentations. labels Oct 5, 2016
@Fishrock123
Copy link
Contributor

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

Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

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
Copy link
Member Author

AnnaMag commented Oct 5, 2016

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

Copy link
Member

@lpinca lpinca left a comment

Choose a reason for hiding this comment

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

LGTM

@fhinkel
Copy link
Member

fhinkel commented Oct 6, 2016

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

@jasnell
Copy link
Member

jasnell commented Oct 6, 2016

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

@addaleax
Copy link
Member

addaleax commented Oct 6, 2016

@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
Copy link
Member Author

AnnaMag commented Oct 6, 2016

@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
Copy link
Member

lpinca commented Oct 6, 2016

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

@AnnaMag
Copy link
Member Author

AnnaMag commented Oct 6, 2016

done! apologies for the confusion:)

@addaleax
Copy link
Member

addaleax commented Oct 6, 2016

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 addaleax self-assigned this Oct 6, 2016
@addaleax
Copy link
Member

addaleax commented Oct 6, 2016

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
Copy link
Member Author

AnnaMag commented Oct 6, 2016

@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
Copy link
Member

addaleax commented Oct 6, 2016

Aye aye then!

Landed this in bf0bcf4, thanks for the contribution!

@addaleax 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 MylesBorins mentioned this pull request Nov 22, 2016
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