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: fix recommendation to use useless config#12445
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
benjamingr commented Apr 16, 2017
Why useless? |
gibfahn commented Apr 16, 2017
See: #11412 |
gibfahn left a comment • 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.
I think it should be git config --global --add apply.whitespace fix, as that fixes whitespace when you apply a patch, which is where a collaborator would need it.
Also a Refs: https://github.com/nodejs/node/issues/11412 in the commit message would be good.
gibfahn commented Apr 16, 2017 • 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.
Please could you format your commit message according to the Contributing Guidelines? Maybe something like this? If you don't have time it can be rewritten by whoever lands this, but if you are going to help out more on the project (as I hope you do), then it saves work if you produce better commit messages. |
matkoniecz commented Apr 17, 2017
I reformatted commit message. |
matkoniecz commented Apr 17, 2017
It makes sense, given "Please never use GitHub's green "Merge Pull Request" button" in https://github.com/nodejs/node/blob/master/COLLABORATOR_GUIDE.md#landing-pull-requests . |
matkoniecz commented Apr 17, 2017
@gibfahn It is ready for rereview. |
mscdex commented Apr 17, 2017 • 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.
Minor nit: there's a typo in the commit message (below the first line). |
Old config instruction instructed people to enable option that does not exist and is silently ignored by git Refs: #11412
matkoniecz commented Apr 17, 2017
Thanks, should be fixed now. |
Use apply.whitespace=fix rather than core.whitespace=fix Ref: #11412 PR-URL: #12445 Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]>
jasnell commented Apr 18, 2017
Landed in def78e8 with a modified commit log message. |
Fishrock123 commented Apr 18, 2017
@jasnell commit did not link? |
jasnell commented Apr 18, 2017 • 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.
That's odd! let's try again with the full hash def78e8 |
gibfahn commented Apr 18, 2017
@jasnell yeah I've noticed it not link properly quite a few times, possibly a GitHub issue. I generally just go to https://github.com/nodejs/node and click on the |
aqrln commented Apr 18, 2017 • 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.
Maybe we are starting to be out of seven-character hashes and there was an ambiguity. FWIW, I copy the full commit hash via UPD: nope, no ambiguity, that's the only commit to start with |
Use apply.whitespace=fix rather than core.whitespace=fix Ref: #11412 PR-URL: #12445 Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]>
Use apply.whitespace=fix rather than core.whitespace=fix Ref: #11412 PR-URL: #12445 Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]>
Use apply.whitespace=fix rather than core.whitespace=fix Ref: #11412 PR-URL: #12445 Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]>
Use apply.whitespace=fix rather than core.whitespace=fix Ref: #11412 PR-URL: #12445 Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]>
Use apply.whitespace=fix rather than core.whitespace=fix Ref: #11412 PR-URL: #12445 Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]>
Use apply.whitespace=fix rather than core.whitespace=fix Ref: nodejs/node#11412 PR-URL: nodejs/node#12445 Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]>
Checklist
Affected core subsystem(s)
doc