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.

Sign up for GitHub

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

Jump to bottom

doc: suggest xcode-select --install #13264

Merged
merged 1 commit into from
Jun 3, 2017
Merged

doc: suggest xcode-select --install #13264

merged 1 commit into from
Jun 3, 2017

Conversation

gibfahn
Copy link
Member

@gibfahn gibfahn commented May 28, 2017

If you don't have the full Xcode installed, the command-line tools are
all you need.

Fixes: #6449
Refs: #9215

cc/ @nodejs/platform-macos

Checklist
Affected core subsystem(s)

doc, build

@gibfahn gibfahn added build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. macos Issues and PRs related to the macOS platform / OSX. labels May 28, 2017
@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. labels May 28, 2017
@gibfahn gibfahn requested review from Trott, targos and mscdex May 28, 2017 11:12
@gibfahn gibfahn mentioned this pull request May 28, 2017
Closed
@gibfahn
Copy link
Member Author

gibfahn commented May 28, 2017

@mscdex cf: #9215 (comment)

How far back does xcode-select --install work for older versions of Xcode? I don't really mind either way, I just thought it might be useful to keep instructions for new and old if the instructions will differ.

It looks like it works from 10.9 (mavericks), so I think we should be fine. IMHO it's unnecessary to go into any more detail, or to consider the full Xcode install for Xcode 7.x, we can always reconsider if someone has an issue.

lpinca
lpinca reviewed May 28, 2017
View reviewed changes
BUILDING.md Outdated
@@ -83,8 +83,10 @@ Prerequisites:

On macOS, you will also need:
* [Xcode](https://developer.apple.com/xcode/download/)
Copy link
Member

Choose a reason for hiding this comment

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

Can we reword this a bit? At the moment it says "On macOS, you will also need -> Xcode -> You also need to install the Xcode Command Line Tools ...".

A full installation of Xcode is not required, so maybe this can be changed to

On macOS, you will also need:

  • The Xcode Command Line Tools ...

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated, PTAL.

@gibfahn gibfahn force-pushed the xcode-tools branch 2 times, most recently from d2bc186 to 93f8ecf Compare May 28, 2017 13:57
Trott
Trott reviewed May 28, 2017
View reviewed changes
BUILDING.md Outdated
On macOS you will need to install the `Xcode Command Line Tools` by running
`xcode-select --install`. If you already have the full Xcode installed you can
find them under the menu `Xcode -> Open Developer Tool -> More Developer
Tools...`. This step will install `clang`, `clang++`, and `make`.
Copy link
Member

Choose a reason for hiding this comment

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

This seems confusing. Should I run the command line or should I open the GUI and "find them"? (I imagine the answer is "either will work" but I'm not sure that's clear from this text. If xcode-select --install will work everywhere all the time, then maybe let's just leave it at that? Or preface the GUI content with "Alternatively, " so that people know it's one or the other?

@Trott
Copy link
Member

Trott commented May 28, 2017

Can someone who doesn't have the full XCode installed confirm that this actually works (that is, they can compile node this way)?

@lpinca
Copy link
Member

lpinca commented May 28, 2017 •
edited
Loading

@Trott can confirm I've always used this way.

@gibfahn
Copy link
Member Author

gibfahn commented Jun 3, 2017 •
edited
Loading

Or preface the GUI content with "Alternatively, " so that people know it's one or the other?

@Trott I went with this. Technically you can install both and switch between them with xcode-select --switch /path, but that's TMI.

LGTY?

Trott
Trott reviewed Jun 3, 2017
View reviewed changes
BUILDING.md Outdated

* After building, you may want to setup [firewall rules](tools/macosx-firewall.sh)
On macOS you will need to install the `Xcode Command Line Tools` by running
`xcode-select --install`. Alternatively if you already have the full Xcode
Copy link
Member

Choose a reason for hiding this comment

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

Nit: comma after Alternatively

Trott
Trott reviewed Jun 3, 2017
View reviewed changes
BUILDING.md Outdated
* After building, you may want to setup [firewall rules](tools/macosx-firewall.sh)
On macOS you will need to install the `Xcode Command Line Tools` by running
`xcode-select --install`. Alternatively if you already have the full Xcode
installed you can find them under the menu `Xcode -> Open Developer Tool ->
Copy link
Member

Choose a reason for hiding this comment

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

Nit: comma after installed

@Trott
Copy link
Member

Trott commented Jun 3, 2017

LGTY?

Text looks good to me. (Left a couple of punctuation nits, but whatever.) I don't know enough about the actual correctness of the content to provide a review/approval. I don't doubt that it's correct, but I'm not knowledgable enough to put a stamp on it. If you and @lpinca both say it's correct, that's good enough for me.

@gibfahn gibfahn force-pushed the xcode-tools branch 3 times, most recently from 8c686ca to 7c3cc26 Compare June 3, 2017 18:39
@gibfahn
doc: suggest xcode-select --install
fc102d0 
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 <luigipinca@gmail.com>
@gibfahn gibfahn merged commit fc102d0 into nodejs:master Jun 3, 2017
@gibfahn gibfahn deleted the xcode-tools branch June 3, 2017 18:44
jasnell pushed a commit that referenced this pull request Jun 5, 2017
@gibfahn @jasnell
doc: suggest xcode-select --install
52c0c47 
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 <luigipinca@gmail.com>
@jasnell jasnell mentioned this pull request Jun 5, 2017
Closed
@gibfahn gibfahn mentioned this pull request Jun 15, 2017
Closed
3 tasks
MylesBorins pushed a commit that referenced this pull request Jul 17, 2017
@gibfahn @MylesBorins
doc: suggest xcode-select --install
ae00f25 
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 <luigipinca@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jul 18, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Trott Trott Trott left review comments

@lpinca lpinca lpinca approved these changes

@targos targos Awaiting requested review from targos

@mscdex mscdex Awaiting requested review from mscdex

Assignees
No one assigned
Labels
build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. macos Issues and PRs related to the macOS platform / OSX.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@gibfahn @Trott @lpinca @MylesBorins @nodejs-github-bot