Xcode 10/Swift 4.2. Use Result 4.0.0 Fix #192 #193
Conversation
|
on travis same issue than previous PR |
|
@Thomvis Given the keynote next week do you think that this should be prioritized? |
| @@ -8,6 +8,17 @@ | |||
|
|
|||
| import Result | |||
|
|
|||
| extension ResultProtocol { | |||
There was a problem hiding this comment.
Is there a reason that we're adding this here? From what I can tell it doesn't look like this needs to be here in order to fix the issue at hand but that's just from a cursory glance at the Result code. If no, it might be better to put this up on the Result repo rather than this one because I do like the addition
There was a problem hiding this comment.
Sorry, I need to learn how to read PR descriptions 😅. I do still think that you should put up a PR for this on Result's repo but this is a nice stop gap measure.
There was a problem hiding this comment.
As you can see, Result framework now encapsulate a Result in a result(resultProtocol)
My others change demonstrate it.
Result framework do not want all this function on the protocol, that’s not the spirit of their modications (composition pattern)
I make it as extension here just to not edit a lot of code, ie. do xxx.result.analysis everywhere
| @@ -8,6 +8,17 @@ | |||
|
|
|||
| import Result | |||
|
|
|||
| extension ResultProtocol { | |||
There was a problem hiding this comment.
Sorry, I need to learn how to read PR descriptions 😅. I do still think that you should put up a PR for this on Result's repo but this is a nice stop gap measure.
|
Hi, @Thomvis |
|
@phimage I've added you as a collaborator. Thanks for taking the time to help out. It would be great if you could spearhead the new Xcode 10 compatible release. |
phimage
changed the title
|
I've merged the PR into master and will cut a release shortly. Thanks for your help, and patience! |
Just restoring missing function
analysisremoved fromResultand fix value and error getters for new
ResultProtocolimplementation in Result 4.0.0