-
Notifications
You must be signed in to change notification settings - Fork 71
Add quick pick to set Xcode DEVELOPER_DIR #384
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
Conversation
@aelam does this work for you? |
@adam-fowler will try it out this week. Thanks for mentioning me ! |
Do you want your real email address in the CONTRIBUTORS.txt? |
Yes! It would be my honor! |
You need to tell me what it is though |
configuration.swiftEnvironmentVariables = swiftEnv; | ||
// if SDK is inside previous DEVELOPER_DIR then move to new DEVELOPER_DIR | ||
if ( | ||
configuration.sdk.length > 0 && |
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.
Here is convenient enough
but I'm wondering if it's possible for the setting of swift.SDK could be like either way
- swift.SDK: "SDKs/MacOSX.sdk" (
DEVELOPER_DIR
is the automatic prefix for it) - swift.SDK: "
$DEVELOPER_DIR
/SDKs/MacOSX.sdk". (environment variable can be used in arguments too)
a bit out of scope here. you may find some ideas here
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.
DEVELOPER_DIR doesn't exist on Linux or Windows. Also the SDK can exist outside the DEVELOPER_DIR if you are cross compiling for another platform.
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.
Thanks for your answer
* Get list of Xcode versions intalled on mac | ||
* @returns Folders for each Xcode install | ||
*/ | ||
public static async getXcodeInstalls(): Promise<string[]> { |
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.
[nits] getInstalledXcodes
may be easier to understand?
await withQuickPick( | ||
selectedXcode ?? defaultXcode, | ||
xcodes.map(xcode => { | ||
const developerDir = `${xcode}/Contents/Developer`; |
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.
[nits] the UI and the actual value is a bit different
the list in UI is Xcode paths
but the actual values are the developer paths
it's a little bit different, not sure if engineers would get confused
I don't see any problems, it's convenient. when I check this PR, I found an issue #387 (not related to this one). Please check |
95e1b59
to
100dc8d
Compare
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.
LGTM
No description provided.