-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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: Update Command Line Tools download procedure #9215
Conversation
Would it be better to keep both but separate them by version, in case someone has the older version of Xcode? For example: On OS X, you will also need:
|
I think we should remove Xcode from the requirements. The minimal required step on OSX is to run |
I had the developer tools installed, but when I tried to build Node.js, it threw an error saying XCode is not installed. I probably had the old version of developer tools though. I ended up updating OSX to install XCode, and I didn't have to install the command line tools separately. @targos @mscdex Going forward, what do you suggest? Should I replace XCode with This is also my first contribution to Node.js. Do you recommend whatever change I make next to be a separate commit, or squashed into one commit? How do you like to do it in Node.js? |
@jeenalee I have the command-line tools installed, and when I run As for separate vs squashed, it's personal preference really. I guess if we're exploring different options it might be useful to add another commit so people can compare... |
How far back does |
The earliest record I could find point to 10.9 for that method. I'd also be fine if it just says |
BUILDING.md
Outdated
@@ -20,8 +20,9 @@ Prerequisites: | |||
|
|||
On OS X, you will also need: | |||
* [Xcode](https://developer.apple.com/xcode/download/) | |||
* You also need to install the `Command Line Tools` via Xcode. You can find | |||
this under the menu `Xcode -> Preferences -> Downloads` | |||
* You also need to install the `Command Line Tools` via Xcode. You |
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.
why did you wrap "can find" to the next line? Was it over 80 columns? It makes the diff harder to read to find the non-stylistic change.
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.
My text editor had 72-character width setting. Sorry, I'll update it.
The instruction on downloading Command Line Tools through XCode has been changed. This patch updates the building documentation to reflect the changes.
56c0543
to
7302b1d
Compare
@nodejs/build ... does this make sense to do? |
I think we should suggest |
There hasn't been any activity here. I'm closing this. Feel free to reopen (or ping a collaborator) if I closed this in error. |
If you don't have the full Xcode installed, the command-line tools are all you need. PR-URL: nodejs#13264 Fixes: nodejs#6449 Refs: nodejs#9215 Reviewed-By: Luigi Pinca <[email protected]>
If you don't have the full Xcode installed, the command-line tools are all you need. PR-URL: #13264 Fixes: #6449 Refs: #9215 Reviewed-By: Luigi Pinca <[email protected]>
If you don't have the full Xcode installed, the command-line tools are all you need. PR-URL: #13264 Fixes: #6449 Refs: #9215 Reviewed-By: Luigi Pinca <[email protected]>
Checklist
Affected core subsystem(s)
doc
Description of change
The way to download Command Line Tools through XCode has been
changed. This patch updates the building documentation to reflect the
changes.