Skip to content

Conversation

babolivier
Copy link
Contributor

@babolivier babolivier commented Jan 21, 2022

This is happening in the context of mainlining the fork of Synapse used for Tchap, and migrates an existing feature from the Tchap fork into a module, leveraging the callback and module API added in matrix-org/synapse#11790.

Most of the code in this module has been copied over from https://github.com/matrix-org/synapse-dinsic/blob/d1fedca096250e04c8f1b7101003f3f4cfddea9a/synapse/rest/client/register.py#L622-L670

CI is expected to fail since the callback and module API used haven't been released in Synapse yet.

@babolivier babolivier added the Z-Time-Tracked Element employees should track their time spent on this issue/PR. label Jan 21, 2022
@babolivier babolivier requested a review from a team as a code owner January 21, 2022 18:32
@DMRobertson
Copy link

CI is expected to fail since the callback and module API used haven't been released in Synapse yet.

Fair, but FWIW I think one of the mypy errors would be fixed by requiring the types-mock package as a dev dependency.

@babolivier
Copy link
Contributor Author

CI is expected to fail since the callback and module API used haven't been released in Synapse yet.

Fair, but FWIW I think one of the mypy errors would be fixed by requiring the types-mock package as a dev dependency.

Ah yeah good point.

Copy link

@DMRobertson DMRobertson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@babolivier babolivier merged commit 8925d74 into main Jan 26, 2022
@babolivier
Copy link
Contributor Author

For the paper trail, I'm merging despite CI failing since failures are due to the reasons mentioned in the PR description. I've ran the tests and linters/mypy locally with the branch from matrix-org/synapse#11790 checked out and everything's passing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Z-Time-Tracked Element employees should track their time spent on this issue/PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants