-
Notifications
You must be signed in to change notification settings - Fork 822
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
Issue #179. Add currentXcodeVersion as an option. #182
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,7 +5,7 @@ import PathKit | |
import Yams | ||
|
||
public struct ProjectSpec { | ||
|
||
public var basePath: Path | ||
public var name: String | ||
public var targets: [Target] | ||
|
@@ -20,6 +20,9 @@ public struct ProjectSpec { | |
public var include: [String] = [] | ||
|
||
public struct Options { | ||
fileprivate static let defaultXcodeVersion = "0910" | ||
|
||
public var currentXcodeVersion: String | ||
public var carthageBuildPath: String? | ||
public var createIntermediateGroups: Bool | ||
public var bundleIdPrefix: String? | ||
|
@@ -47,7 +50,8 @@ public struct ProjectSpec { | |
} | ||
} | ||
|
||
public init(carthageBuildPath: String? = nil, createIntermediateGroups: Bool = false, bundleIdPrefix: String? = nil, settingPresets: SettingPresets = .all, developmentLanguage: String? = nil) { | ||
public init(currentXcodeVersion: String? = nil, carthageBuildPath: String? = nil, createIntermediateGroups: Bool = false, bundleIdPrefix: String? = nil, settingPresets: SettingPresets = .all, developmentLanguage: String? = nil) { | ||
self.currentXcodeVersion = currentXcodeVersion ?? Options.defaultXcodeVersion | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you think most users of this option will know to use the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I personally think that’d be a matter of documentation, most users shouldn’t even need to set this, having multiple formats could be overdoing it IMHO. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I agree with @rahul-malik here. If someone defines a string with a single |
||
self.carthageBuildPath = carthageBuildPath | ||
self.createIntermediateGroups = createIntermediateGroups | ||
self.bundleIdPrefix = bundleIdPrefix | ||
|
@@ -119,7 +123,8 @@ extension ProjectSpec: Equatable { | |
extension ProjectSpec.Options: Equatable { | ||
|
||
public static func == (lhs: ProjectSpec.Options, rhs: ProjectSpec.Options) -> Bool { | ||
return lhs.carthageBuildPath == rhs.carthageBuildPath && | ||
return lhs.currentXcodeVersion == rhs.currentXcodeVersion && | ||
lhs.carthageBuildPath == rhs.carthageBuildPath && | ||
lhs.bundleIdPrefix == rhs.bundleIdPrefix && | ||
lhs.settingPresets == rhs.settingPresets && | ||
lhs.createIntermediateGroups == rhs.createIntermediateGroups && | ||
|
@@ -158,6 +163,7 @@ extension ProjectSpec { | |
extension ProjectSpec.Options: JSONObjectConvertible { | ||
|
||
public init(jsonDictionary: JSONDictionary) throws { | ||
currentXcodeVersion = jsonDictionary.json(atKeyPath: "currentXcodeVersion") ?? ProjectSpec.Options.defaultXcodeVersion | ||
carthageBuildPath = jsonDictionary.json(atKeyPath: "carthageBuildPath") | ||
bundleIdPrefix = jsonDictionary.json(atKeyPath: "bundleIdPrefix") | ||
settingPresets = jsonDictionary.json(atKeyPath: "settingPresets") ?? .all | ||
|
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.
Can we name this just
xcodeVersion
. You probably just went with the old name, sure why the old property wascurrentXcodeVersion
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.
Can we also make this an optional and move the
defaultXcodeVersion
into XcodeGenKit. I'd prefer to keep any hardcoded values out of this module, as some people use it independently and it's really the generator that supports a certain version or not.