Xcode 10 & Swift 4.2 #683
Xcode 10 & Swift 4.2 #683
Conversation
Generated by 🚫 Danger |
There was a problem hiding this comment.
Hi @Landsi 👋
I've skimmed over the changes and have some questions. Could you look at them please?
| pod 'StencilSwiftKit', '2.6.0' | ||
| pod 'Commander', '0.7.0' | ||
| pod 'PathKit', '0.9.2' | ||
| pod "xcproj", :git =>'[email protected]:tuist/xcodeproj.git', :tag => '4.3.1' |
There was a problem hiding this comment.
What is the reason you use a fork instead of https://github.com/xcodeswift/xcproj.git here? Because you're using the main repo in the SPM manifest 🤔
There was a problem hiding this comment.
xcodeswift/xcproj.git redirects to tuis/xcodeproj.git
There was a problem hiding this comment.
it's a shame that they didn't release the 4.3.1 version properly with cocoapods..
| // TODO: seems like the inherited types are not always in the same order (any more) | ||
| // expect(bazType.allVariables[1].definedInType).to(equal(fooType)) | ||
| // expect(bazType.allVariables[2].definedInType).to(equal(barType)) | ||
| expect(bazType.allVariables.map({$0.definedInType})).to(contain([fooType, barType])) |
There was a problem hiding this comment.
I suppose the order here is not critical. But maybe you could add another check on the count attribute to ensure that there are just theses types in definedInType?
There was a problem hiding this comment.
| expect(result).to(equal("(\n Hello,\n beautiful,\n World\n)")) | ||
| // TODO: apparently something changed in the `toArray` filter? | ||
| // expect(result).to(equal("(\n Hello,\n beautiful,\n World\n)")) | ||
| expect(result).to(equal("[Hello, beautiful, World]")) |
There was a problem hiding this comment.
🤔 either the previous expectation was incorrect (which I doubt) or we have a major change of how loops on dictionaries are done in Stencil?
This PR enables Xcode 10 support and paves the way for #680.
This will not build with Xcode 9.x any more, due to different handling of module maps in combination with cocoapods.
I ran a full
pod updateon this (yes i dared @krzysztofzablocki - nice plugin ;) )I kept your scheme of specifying specific pod versions, although i believe it would be better to be a bit more lenient on the patch version at least, if not adopting semantic versioning altogether.
I updated the pods to already support swift 4.2 (i think, haven't really tried), but did not update the sourcery project itself.
2 tests failed after the update. i believe it's nothing major, but check out my changes in the specs before merging. now the tests succeed but my changes in the specs files might not be correct.
cheers, max