Skip to content
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

fix: Switch to iOS 13, use Tasks to handle threads and add error handling #899

Closed
wants to merge 8 commits into from

Conversation

juliansteenbakker
Copy link
Owner

Continuation of #726

ened and others added 2 commits August 9, 2023 07:55
# Conflicts:
#	example/ios/Podfile
#	ios/Classes/MobileScanner.swift
#	ios/Classes/MobileScannerPlugin.swift
@juliansteenbakker juliansteenbakker changed the title Continuation: Switch to iOS 13, use Tasks to handle threads and add error handling fix: Switch to iOS 13, use Tasks to handle threads and add error handling Dec 18, 2023
@yeras-is
Copy link

Any updates on this PR ? @juliansteenbakker

@juliansteenbakker
Copy link
Owner Author

Since this PR needs a minimum of iOS 13, i am working on a implementation that also enabled iOS 12 to work. @yeras-is

@juliansteenbakker juliansteenbakker marked this pull request as ready for review December 19, 2023 13:57
@juliansteenbakker
Copy link
Owner Author

I have got this working right now, however, im not yet convinced if this is an improvement over the existing features. I'm not quite familiar with swift async programming and completion, so i can't really point out the advantage over each other. @navaronbracke do you have any idea's about if this change is really an improvement, or will just clutter the existing codebase more?

@navaronbracke
Copy link
Collaborator

I'm not that familiar with Tasks in Swift, but a quick search yields the following comment as a good summary:

https://developer.apple.com/forums/thread/712303?answerId=724093022#724093022

Since we are only using the task for the start() method, it's up to you to decide if we want it or not.
I'm fine with either option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants