-
Notifications
You must be signed in to change notification settings - Fork 33
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
Use Zbarlight as alternative to pyzbar when it's not available #42
Conversation
Thanks for giving it a look and spending time on it. |
@AndreMiras sure thing :-) I'll look into both, thanks for your early feedback! |
I quickly looked at the failing tests - I thought it might be a good idea to normalize the code_types as lowercase because that used to be the case, but obviously people might rely on them as they are now :-) fixing! |
TODO: Fix setup.py install/extra_requires accordingly, as well as requirements in buildozer.spec for demo app
Alright, tests should succeed, and I think this generally should work as intended :-) |
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.
I think this is looking good, well done.
I wish you wrote additional tests for some of the new behaviour so nobody breaks it without noticing.
I'll merge still (and fix the linting) feel free to follow up with increase test coverage
✅ Also adds tests This is a follow up for #42
Made the follow up PR with tests. Also introduced a class for the cross implementation to ease testing and avoid root level logic #46 |
This is a follow up for #42.
Everything in the title - pyzbar won't work on iOS.
EDIT: FWIW this PR is rebased onto #39 - it's probably a good idea to merge both, but if that's considered problematic I can probably split them.