-
Notifications
You must be signed in to change notification settings - Fork 126
Additional exception fields #288
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
Conversation
LGTM regarding #208 . |
This is out of my depth. |
Any ideas on a review? Without this it's impossible to have more than one operation pending on a connection. |
My review will be sub-par, but it looks like I'll need to do it to move this forward. I'll dedicate a day next week to python-ldap. |
Thanks, I'll rebase it in the meantime. |
82f050b
to
8492b16
Compare
Only fails on the same doctest as master now. |
The last commit should go into a separate PR:
|
I've moved the WIP commit into its own branch, but kept the |
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.
Looks good!
Thank you for the change, and apologies for the delay in reviewing.
Work to improve how we raise exceptions on result, deals with controls and msgid being missing (#177, #208).
Ready for merging (pending #282, #283, #284 have been accepted) apart from last commit which is mainly here for initial review and can be split into its own PR if you want.