-
Notifications
You must be signed in to change notification settings - Fork 315
Bump Ruma (breaking) #5337
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
base: main
Are you sure you want to change the base?
Bump Ruma (breaking) #5337
Conversation
9730498
to
d05861b
Compare
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #5337 +/- ##
==========================================
+ Coverage 88.81% 88.83% +0.01%
==========================================
Files 334 333 -1
Lines 91261 91136 -125
Branches 91261 91136 -125
==========================================
- Hits 81054 80959 -95
+ Misses 6396 6367 -29
+ Partials 3811 3810 -1 ☔ View full report in Codecov by Sentry. |
d05861b
to
dc253ad
Compare
dc253ad
to
8423330
Compare
// Safety: A decrypted event always includes a room_id in its | ||
// payload. |
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.
I wouldn't put Safety:
if it's not about unsafe
code.
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.
I copied this comment from other places in the code that do the same thing.
None, | ||
rendezvous_server.to_string(), | ||
None, | ||
&SupportedVersions { versions: Default::default(), features: Default::default() }, |
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.
Should we have an associated const
for empty SupportedVersions
upstream?
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.
In this case, it looks to me like we should call /versions
first, otherwise when this becomes stable we won't know which version of the path to call.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
…oints Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
8423330
to
93e2549
Compare
I think you can rebase with |
This is more or less ready, with all the changes that are required due to breaking changes in Ruma. I still need to fix a few tests. I will probably open other PRs soon to try to make this slightly smaller (but there is not much to do).
I tried to split the commits by source of the change, so I recommend to review this commit by commit. However after the first commit the code doesn't compile unless all the other commits are applied, so I recommend to squash everything when merging.
Note that currently Ruma is not ready for a release with those breaking changes, however it should be ready in a few weeks.