-
Notifications
You must be signed in to change notification settings - Fork 129
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
RUMM-669 Xcode 11.3.1 compilation fix #217
Conversation
b1425c4
to
0c6a12a
Compare
let data = fileHandle.readDataToEndOfFile() | ||
try? objcExceptionHandler.rethrowToSwift { | ||
fileHandle.closeFile() | ||
} | ||
return data |
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.
nit: we can keep its original order
let data = fileHandle.readDataToEndOfFile() | |
try? objcExceptionHandler.rethrowToSwift { | |
fileHandle.closeFile() | |
} | |
return data | |
defer { | |
try? objcExceptionHandler.rethrowToSwift { | |
fileHandle.closeFile() | |
} | |
} | |
fileHandle.readDataToEndOfFile() |
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.
previous code was closing fileHandle
in defer
block, this is the original order as readDataToEndOfFile
doesn't throw and closeFile
is called after reading
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.
You're right. I updated my suggestion. Anyway, it does the same as the code in PR 👍.
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.
yes, it does the same thing, i go with shorter code 🩳
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 do we need all these changes in swizzling code?
@ncreated i changed the order of parameters in other Xcode versions don't have this issue |
Xcode 11.3.1 doesn't have iOS 13.4 as base SDK We added compiler version macros as workaround
0c6a12a
to
4402e4f
Compare
What and why?
Xcode 11.3.1 doesn't have iOS 13.4 as base SDK
therefore
#available(iOS 13.4)
closures don't compileFor more: #214
How?
We added compiler version macros as workaround
Note: It's not trivial to run our tests in different Xcode versions with our CI provider
Release info
This PR is branched off from
1.3.0
tag and to be merged intomaster
We will ship this change as a hotfix once it is merged
Review checklist