compas-dev / compas_eve

COMPAS Event Extensions: adds event-based communication infrastructure to the COMPAS framework.
http://compas.dev/compas_eve/
MIT License
3 stars 1 forks source link

Fix several issues #12

Closed gonzalocasas closed 4 months ago

gonzalocasas commented 4 months ago

Solves https://github.com/compas-dev/compas_eve/issues/11

This PR fixes several things: first of all, the base case of the Message class was removed and re-based to object because UserDict is an old-style class and that creates a ton of problems.

What type of change is this?

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

gonzalocasas commented 4 months ago

/cc @jckenny59

gonzalocasas commented 4 months ago

Generally LTGM, but I am a bit confused by some things. Mostly the (de)serialization of Message objects, just seems like a parallel system to the compas.data. But if it's nonsense feel free to go ahead :)

Thanks! I think I fixed all the review findings. About the (de)serialization of Message, the idea is not to go entirely parallel to compas.data, but to support it without forcing it to be the case. So, a message can be an instance of Message without implementing compas.data.Data as well. Ideally, it's also possible to send/receive even a simple dictionary.