Refactor describe(), add llbuild tool types #175
Conversation
| extension Array: YAMLRepresentable { | ||
| var YAML: String { | ||
| let stringArray = self.flatMap { $0 as? String } | ||
| guard stringArray.count == self.count else { return "" } |
There was a problem hiding this comment.
Shouldn't we throw an error here if array has non String elements?
like, fatalError("Unimplemented YAML type")
There was a problem hiding this comment.
probably yeah. Actually I was trying to write something like "Array conforms to YAMLRep where Element = String" but couldn't get that to work. Not sure if it's possible right now?
There was a problem hiding this comment.
How about extension Array where Element: YAMLRepresentable
That's not the same, but that should work in your use-case. Array doesn't confirm to YAMLRepresentable, but you still can call YAML method on array where elements are YAMLRepresentable like
["-", "1", "2", "3"].YAML // "["-", "@", 1, 2, 3]"
[1, 2, 3].YAML //Compile ErrorThere was a problem hiding this comment.
I thought about it when I was doing it but as you said Array will not conform to YAMLRepresentable but it will work for the current use case and provide better compile time checks. Updating code.
aciidgh
force-pushed
the
describe_refactor
branch
from
07e760b to
2dfd20a
Compare
|
LGTM. @mxcl can you trigger CI-tests? |
|
@mxcl CI pls |
|
@swift-ci Please test |
aciidgh
force-pushed
the
describe_refactor
branch
from
2dfd20a to
51d170c
Compare
|
CI failed due to |
|
@swift-ci Please test |
Refactor describe(), add llbuild tool types
|
In future please wait for my review before merging non-trivial changes. Although in this case I would have said: great work 👍🏻 |
|
Oh, so far I thought you only triggered CI if you had reviewed. Will wait for explicit comment in future. |
Fix `BuildSystemTaskTests`
This PR adds models for the llbuild tools used by swift-build and method which can take array of llbuild Targets to produce YAML.