Skip to content
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

Exhaustive sync_events::Response conversion #209

Merged
merged 3 commits into from
Apr 19, 2021
Merged

Exhaustive sync_events::Response conversion #209

merged 3 commits into from
Apr 19, 2021

Conversation

jplatte
Copy link
Collaborator

@jplatte jplatte commented Apr 19, 2021

Unfortunately I had to duplicate the entire destructuring statement for sync_events::Response since attributes on field patterns or .. rest-field patterns are not supported. (cc rust-lang/rust#81282 (comment))

From here it should be easy to add EncryptionInfo as discussed without further changes to Ruma, right? If so, this closes ruma/ruma#35.

@jplatte
Copy link
Collaborator Author

jplatte commented Apr 19, 2021

This whole thing would look less ugly once it's more directly supported with a lint that triggers on reachable rest patterns. I've opened rust-lang/rust#84332 for that.

So the SDKs own SyncResponse type doesn't get out-of-sync.
@jplatte
Copy link
Collaborator Author

jplatte commented Apr 19, 2021

This probably obsoletes #136 too.

@poljar
Copy link
Contributor

poljar commented Apr 19, 2021

This probably obsoletes #136 too.

Well yes and no, yes in its current form. We still need a PR to include the encryption info and what not.

@jplatte
Copy link
Collaborator Author

jplatte commented Apr 19, 2021

Yes, of course :)

Should I open an issue about that, as a replacement for ruma/ruma#35 and #136?

@poljar
Copy link
Contributor

poljar commented Apr 19, 2021

I mean sure if you want to, though I can also just keep that in my head since I'll work on #136 this week.

@jplatte
Copy link
Collaborator Author

jplatte commented Apr 19, 2021

Okay, then I won't create an issue. I'm done force-pushing by the way 😅

@poljar poljar merged commit 65d84c1 into matrix-org:master Apr 19, 2021
@poljar
Copy link
Contributor

poljar commented Apr 19, 2021

Thanks, merged.

@jplatte jplatte deleted the exhaustive-sync-events-conv branch April 19, 2021 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Events should have attributes indicating the encryption state.
2 participants