-
-
Notifications
You must be signed in to change notification settings - Fork 579
Re-add PartialEq removed in #230 #232
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
Deploying with
|
Latest commit: |
694ba90
|
Status: | ✅ Deploy successful! |
Preview URL: | https://3ebb18a8.graphite-master.pages.dev |
wait a sec |
We should evaluate it there is a good reason to require partial eq before just removing it |
Ok. I gave my approval but let's hold off until we can determine if this is the best course of action. |
We will need an eq implementation for message deduplication which is why I initially put it there |
We can have a discussion on if we can avoid requiring partial eq for that but it is a discussion we should have |
When would messages be duplicated? |
For example if you have multiple requests to update the canvas for example, you obviously only need to process the last one |
940095c
to
694ba90
Compare
Summary from the discussion on Discord: From @TrueDoctor:
I have force-pushed a commit which changes this PR to do that. |
Remove more partialeq derives from the messaging system.(Edit: see last comment.) Unblocks #220This change is