-
Notifications
You must be signed in to change notification settings - Fork 35
Remove assert
's
#85
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
Remove assert
's
#85
Conversation
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.
Thanks! These are not really assertion failures, in many cases, but RuntimeError
s or similar, because they do not reflect an error in the code (which is what assertions are checking). So could you consider what exception you would like to be raised for these? The wifi exceptions for similar problems could be used as a model, if they make sense.
This was a minimally invasive pr, however since requested, I will update the exceptions based on |
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.
These changes look good to me! The exceptions chosen are appropriate.
Updated to fix conflicts & to switch |
Is this good to go now that changes from |
Yep, its done. |
Updating https://github.com/adafruit/Adafruit_CircuitPython_Wiznet5k to 1.13.0 from 1.12.18: > Merge pull request adafruit/Adafruit_CircuitPython_Wiznet5k#85 from bill88t/main Updating https://github.com/adafruit/Adafruit_CircuitPython_Bundle/circuitpython_library_list.md to NA from NA: > Updated download stats for the libraries
This is following #67 and is independent to it.
Closes #83 .