-
Notifications
You must be signed in to change notification settings - Fork 734
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
Update to Xcode 9.3 #415
Update to Xcode 9.3 #415
Conversation
.ruby-version
Outdated
@@ -1 +1 @@ | |||
2.4.2 | |||
2.4.4 |
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.
Same remark as for the other PR regarding system version
@@ -80,7 +80,7 @@ extension AssetsCatalogParser { | |||
This method recursively parses a directory structure, processing each folder (files are ignored). | |||
*/ | |||
func process(folder: Path, withPrefix prefix: String = "") -> [Catalog.Entry] { | |||
return (try? folder.children().sorted(by: <).flatMap { | |||
return (try? folder.children().sorted(by: <).compactMap { |
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.
Not sure I get this change
folder.children().sorted(by:<)
is an array
the closure calls process
which returns an array too
So in this use case we should be using the monadic flatMap (the one that wasn't renamed), not the non-monadic [T].compactMap(f: (T) -> T?) -> [T]
one? So why would the compiler want us to change it there, did we get types mixed up and didn't do what we thought we were doing before, using the non monadic one all along by mistake?
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.
The process(item: $0, withPrefix: prefix)
that's called in the compactMap
closure has as type:
func process(item: Path, withPrefix prefix: String) -> Catalog.Entry?
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.
Oh right it's not the same process
method as process(folder: withPrefix: ) -> [Catalog.Entry]
, right. 👍
@@ -56,7 +56,7 @@ extension CTFont { | |||
let descs = CTFontManagerCreateFontDescriptorsFromURL(file.url as CFURL) as NSArray? | |||
guard let descRefs = (descs as? [CTFontDescriptor]) else { return [] } | |||
|
|||
return descRefs.flatMap { desc -> Font? in | |||
return descRefs.compactMap { desc -> Font? in |
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.
(… As compared to here where the change is justified as we operate on an array but the closure returns an optional, so here it's indeed intended to be the non-monadic compactMap
and this change is expected)
I'm assuming that with the ruby change this PR should be ok, and go ahead and merge it. |
Note that to fix some warnings, we have to use
compactMap
in the SwiftGenKit code, which might be considered a breaking change (Swift 4.1). I don't know if this warrants a major version bump for SwiftGenKit (and SwiftGen).