-
Notifications
You must be signed in to change notification settings - Fork 15
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
Decouple frozendict support from the library #59
Conversation
8ef8cab
to
060369e
Compare
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 like a good approach!
Thanks. I'll hold off from merging until I've tested this with Synapse. |
Co-authored-by: Patrick Cloke <clokep@users.noreply.github.com>
A preserialisation hook allows you to encode objects which aren't encodable by the | ||
standard library ``JSONEncoder``. |
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.
Did you want to update the version or changelog in this PR or will do it separate?
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.
Was going to leave it---I think we usually make bumping the version number part of a release
I tested Synapse against this in matrix-org/synapse@266370c and matrix-org/synapse@03e63ab. This passed trial and sytest, see https://github.com/matrix-org/synapse/actions/runs/4419129723/jobs/7747311770 (The other failures are due to me using a |
Commitwise reviewable.
Effectively fixes #58, by allowing consumers (Synapse) to register a preserialisation callback for immutabledict.
Breaking change; will need a semver major bump. Consumers can add back support for frozendict by registering a preserialisation callback.
Synapse 1.62 started requiring semver-bounds on canonicaljson in matrix-org/synapse#13082, and I couldn't see anything else which imports canonicaljson on my machine. So only brand-new PyPI installations of Synapse 1.61 or earlier should be broken by this change. (Indeed, the point of that PR was exactly to allow us to make this kind of change.)