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: typo in api/process.md #12565

Closed
mscdex opened this issue Apr 21, 2017 · 6 comments
Closed

doc: typo in api/process.md #12565

mscdex opened this issue Apr 21, 2017 · 6 comments
Labels
doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors. process Issues and PRs related to the process subsystem.

Comments

@mscdex
Copy link
Contributor

mscdex commented Apr 21, 2017

  • Version: master, v7.x, v6.x
  • Platform: n/a
  • Subsystem: doc

In doc/api/process.md, there is a typo in the 'A note on process I/O' section where it says:

Synchronous writes avoid problems such as output written with `console.log()` or `console.write()` being unexpectedly interleaved, ...

where console.write() should presumably instead be console.error(), since console.write() does not exist and console.error() is the only other console method referred to in that section.

@mscdex mscdex added doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors. process Issues and PRs related to the process subsystem. labels Apr 21, 2017
@mscdex mscdex changed the title doc: typo in doc/api/process.md doc: typo in api/process.md Apr 21, 2017
@morrme
Copy link
Contributor

morrme commented Apr 21, 2017

I can make this change.

@morrme
Copy link
Contributor

morrme commented Apr 22, 2017

I have this ready but it's blocked by another PR :-(. Once that one clears I can send this one.

@gibfahn
Copy link
Member

gibfahn commented Apr 23, 2017

@morrme what do you mean by blocked? You can have lots of PRs open at the same time.

I see that your issue12565 branch also has the commits from your doc-ci-jobs branch. The fix is to do:

git fetch --all # Make sure you have the latest changes
git checkout issue12565
git push # Make sure your remote branches are up to date
git reset --hard upstream/master # Assumes nodejs/node remote is called upstream
git cherry-pick origin/issue12565 # Assumes morrme/node remote is called origin
git push --force-with-lease

The cherry-pick will just pull the top commit from your remote branch, which is the only one you want.

Then you can raise another Pull Request with: master...morrme:issue12565

@morrme
Copy link
Contributor

morrme commented Apr 23, 2017

@gibfahn that's what' I meant. I am so embarrassed to have made such a mistake!

I found a similar fix but we've gotten so far with the CI jobs issue, I was worried about messing it up.

I will give it a try.

Thanks for your support, as always!

@gibfahn
Copy link
Member

gibfahn commented Apr 23, 2017

I'm so embarrassed to have made such a mistake!

Git is hard, no need to be embarassed 😁 .

After the cherry-pick you can do git log --graph --decorate --oneline, you should see something like this (your branch is master + the doc commit):

image

I found a similar fix but we've gotten so far with the CI jobs issue, I was worried about messing it up.

That's a very reasonable thing to worry about. The trick is to run git status before you make changes (make sure you're on the right branch), and to triple-check before your force push (git push -f).

@morrme
Copy link
Contributor

morrme commented Apr 23, 2017

THANK YOU @gibfahn ! 🏆

evanlucas pushed a commit that referenced this issue Apr 25, 2017
PR-URL: #12612
Fixes: #12565
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: David Cai <davidcai1993@yahoo.com>
evanlucas pushed a commit that referenced this issue May 1, 2017
PR-URL: #12612
Fixes: #12565
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: David Cai <davidcai1993@yahoo.com>
evanlucas pushed a commit that referenced this issue May 2, 2017
PR-URL: #12612
Fixes: #12565
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: David Cai <davidcai1993@yahoo.com>
gibfahn pushed a commit that referenced this issue May 16, 2017
PR-URL: #12612
Fixes: #12565
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: David Cai <davidcai1993@yahoo.com>
MylesBorins pushed a commit that referenced this issue May 18, 2017
PR-URL: #12612
Fixes: #12565
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: David Cai <davidcai1993@yahoo.com>
andrew749 pushed a commit to michielbaird/node that referenced this issue Jul 19, 2017
PR-URL: nodejs/node#12612
Fixes: nodejs/node#12565
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: David Cai <davidcai1993@yahoo.com>
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. good first issue Issues that are suitable for first-time contributors. process Issues and PRs related to the process subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants