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

Data overflow fixes #323

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

guusbertens
Copy link

Correctly raise DataOverflowError and display a nicer message on the command line. Comments are welcome!

Fixes issues #273 and #322.

@guusbertens guusbertens marked this pull request as ready for review March 31, 2023 16:44
@guusbertens
Copy link
Author

One of my commits replaces self.version with self._version. While this is not strictly necessary, it makes it so that QRCode.make() calls best_fit by itself, without relying on the getter QRCode.version. The call to best_fit in the getter fn ensures the call to best_fit in make will never be used, which seems unintended. This also makes any exceptions thrown come from a more logical place.

@SmileyChris, what do you think? Should the getter fn modify QRCode's state, even though it's a getter function? I'm mostly curious, but if needed can update this PR.

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.

None yet

1 participant