Skip to content

New defaults module for build_swift argument parser #11911

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

Merged
merged 2 commits into from
Sep 14, 2017

Conversation

Rostepher
Copy link
Contributor

@Rostepher Rostepher commented Sep 14, 2017

Purpose

This PR is yet another small portion of code for staging #11827 and #11872. It adds a new defaults module to build_swift that will hold all default values of significant importance which ought be editable from a single source and easily viewable. There's no functional difference with this PR and should be good to just merge once the tests have passed.

rdar://problem/34425097

@Rostepher
Copy link
Contributor Author

@swift-ci please smoke test

@Rostepher Rostepher closed this Sep 14, 2017
@Rostepher Rostepher reopened this Sep 14, 2017
@Rostepher
Copy link
Contributor Author

@swift-ci please smoke test

@Rostepher Rostepher changed the title Added a new defaults module for configurable constant values used by … New defaults module for build_swift argument parser Sep 14, 2017
Copy link
Contributor

@gottesmm gottesmm left a comment

Choose a reason for hiding this comment

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

Quick question. Looks good to me otherwise.

DARWIN_DEPLOYMENT_VERSION_WATCHOS = '2.0'

UNIX_INSTALL_PREFIX = '/usr'
DARWIN_INSTALL_PREFIX = ('/Applications/Xcode.app/Contents/Developer/'
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed that some of the options here are not used below. Why are they here? Specifically UNIX_INSTALL_PREFIX or am I missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They will be used very soon. I just didn't want to add more complexity in #11827.

@Rostepher
Copy link
Contributor Author

@swift-ci please smoke test

@Rostepher Rostepher force-pushed the argparser-defaults-module branch from 3752897 to f4ce818 Compare September 14, 2017 18:05
@Rostepher
Copy link
Contributor Author

@swift-ci please smoke test

@najacque
Copy link
Contributor

@swift-ci Please nominate

Explanation: This is required staging for the larger task of refactoring build-script argument parsing logic into separate module
Scope: adds a new defaults module to build_swift, no functional difference
Radar: rdar://problem/34425097
Risk: Low
Testing: CI testing

@Rostepher Rostepher merged commit 9847f6a into swiftlang:master Sep 14, 2017
@Rostepher Rostepher deleted the argparser-defaults-module branch September 14, 2017 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants