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

First cut of the module #1

Merged
merged 9 commits into from
Feb 3, 2022
Merged

First cut of the module #1

merged 9 commits into from
Feb 3, 2022

Conversation

babolivier
Copy link
Contributor

@babolivier babolivier commented Feb 1, 2022

First cut of a module to check whether 3pids can be registered against a homeserver based on the response from a custom endpoint. This is happening in the context of the Tchap Synapse mainlining, and is mostly a copy of https://github.com/matrix-org/synapse-dinsic/blob/dinsic/synapse/util/threepids.py#L47-L71

CI is expected to fail since matrix-org/synapse#11854 and matrix-org/synapse#11868 haven't been released in a stable version of Synapse yet. I have ran the tests locally against a branch of Synapse that includes both of these changes, and they're passing.

@babolivier babolivier added the Z-Time-Tracked Element employees should track their time spent on this issue/PR. label Feb 1, 2022
@babolivier babolivier requested a review from a team February 1, 2022 10:35
Copy link
Member

@clokep clokep left a comment

Choose a reason for hiding this comment

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

Looks good overall! A few minor comments.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
setup.cfg Outdated Show resolved Hide resolved
tests/__init__.py Show resolved Hide resolved
tests/test_checker.py Outdated Show resolved Hide resolved
threepid_checker/__init__.py Outdated Show resolved Hide resolved
threepid_checker/__init__.py Outdated Show resolved Hide resolved
threepid_checker/__init__.py Show resolved Hide resolved
@babolivier babolivier requested a review from clokep February 3, 2022 10:38
Copy link
Member

@clokep clokep left a comment

Choose a reason for hiding this comment

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

Looks good! Switching to unittest.mock would be nice, but not a deal breaker!

README.md Outdated Show resolved Hide resolved
Co-authored-by: Patrick Cloke <[email protected]>
@babolivier babolivier merged commit 3bff5d5 into main Feb 3, 2022
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