-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add Yaml support #5
Conversation
1a445d8
to
7aa378f
Compare
README.md.orig
Outdated
@@ -0,0 +1,193 @@ | |||
# PackageGenerator | |||
|
|||
<<<<<<< HEAD |
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.
looks like merge conflict file
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.
🤦♂️
Sources/Core/SpecGenerator.swift
Outdated
|
||
let mappedDependencies: [Spec.RemoteDependency] = spec.remoteDependencies | ||
.compactMap { remoteDependency -> Spec.RemoteDependency? in | ||
guard let dependency = dependencies.dependencies.first(where: { |
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.
broken indent because guard
in the .compactMap
block. and return
(line 40) below looks correct
Sources/Core/SpecGenerator.swift
Outdated
return nil | ||
} | ||
return Spec.RemoteDependency( | ||
name: dependency.name, |
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.
ditto
|
||
struct Product: Decodable { | ||
let name: String | ||
let productType: 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.
Why not replace String
to ProductType
, as a result you will remove custom decoding, and also it will be typesafe to use productType
The same in Target
below
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.
I think it's beyond this PR but I'll take a look.
|
||
init(name: String, url: String, version: String?, revision: String?, branch: String?) { | ||
guard version != nil || revision != nil || branch != nil else { | ||
fatalError("You need to provide at least one of the following: version, revision or branch") |
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.
I think we need to add the same for init(from decoder: Decoder) throws {
if those variable mean the same maybe better to use non-option enum?
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.
I thought the same but I thought it was beyond the scope of this PR.
Regarding the 2 comments on using Decodable better, I tried but it seems that the reason why it wasn't done before is that Stencil doesn't allow fetching properties from computed vars on objects passed in the context. I'm therefore keen in leaving it as it is. Otherwise, this should work: enum Ref: Decodable {
case version(String)
case revision(String)
case branch(String)
enum CodingKeys: String, CodingKey {
case version
case revision
case branch
}
init(from decoder: Decoder) throws {
let container = try decoder.container(keyedBy: CodingKeys.self)
if let version = try container.decodeIfPresent(String.self, forKey: .version) {
self = .version(version)
} else if let revision = try container.decodeIfPresent(String.self, forKey: .revision) {
self = .revision(revision)
} else if let branch = try container.decodeIfPresent(String.self, forKey: .branch) {
self = .branch(branch)
} else {
throw DecodingError.dataCorruptedError(forKey: CodingKeys.version, in: container, debugDescription: "Expected one of version, revision, or branch to be present.")
}
}
} struct Dependency: Decodable {
let name: String
let url: String
let ref: Ref
enum CodingKeys: String, CodingKey {
case name
case url
}
init(from decoder: Decoder) throws {
let container = try decoder.container(keyedBy: CodingKeys.self)
self.name = try container.decode(String.self, forKey: .name)
self.url = try container.decode(String.self, forKey: .url)
self.ref = try Ref(from: decoder)
}
init(name: String, url: String, ref: Ref) {
self.name = name
self.url = url
self.ref = ref
}
} struct Spec: Decodable {
struct Product: Decodable {
let name: String
let productType: ProductType
let targets: [String]
var type: String {
productType.rawValue
}
enum CodingKeys: CodingKey {
case name
case productType
case targets
}
}
...
struct RemoteDependency: Decodable {
let name: String
let url: String?
let ref: Ref?
var version: String? {
if case let .version(value) = ref {
return value
}
return nil
}
var revision: String? {
if case let .revision(value) = ref {
return value
}
return nil
}
var branch: String? {
if case let .branch(value) = ref {
return value
}
return nil
}
enum CodingKeys: CodingKey {
case name
case url
}
init(from decoder: Decoder) throws {
let container = try decoder.container(keyedBy: CodingKeys.self)
self.name = try container.decode(String.self, forKey: .name)
self.url = try container.decodeIfPresent(String.self, forKey: .url)
self.ref = (try? Ref(from: decoder)) ?? nil
}
init(name: String, url: String?, ref: Ref) {
self.name = name
self.url = url
self.ref = ref
}
}
...
|
The changes in this PR are not backward compatible, hence leading to a major version bump (v2).
generate-packages
command as the tool shouldn't make assumption on the environment. Thegenerate-package
command is enough and any logic on top should be implemented in the user space.generate-package
command: the options are now clearer and the command doesn't expect the spec file to be named as the containing folder.jpsim/Yams
..copy
intestTarget.resources
which sorts some warnings.