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

Remove methods argument to verification #1206

Merged
merged 2 commits into from
Feb 13, 2020

Conversation

bwindels
Copy link
Contributor

@bwindels bwindels commented Feb 11, 2020

Fixes element-hq/element-web#12319
It's easy for all the calls in react-sdk to requestVerification(DM) to have a different set of methods they pass along. There is also little value to it, as clients built on top of js-sdk can already select the subset of methods they can provide an UI for through the verificationMethods option when creating the client.

This is sort of a breaking change, as the extra argument consumers of the js-sdk might still pass will be ignored.

as it's easy to have this argument be out of sync from all
the places this is called from the js-sdk. There is also little point,
as you can already specify the methods a consumer of the js-sdk
wants to provide through the verificationMethods option when creating
the client object.
Copy link
Collaborator

@jryans jryans left a comment

Choose a reason for hiding this comment

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

Seems reasonable, thanks! 😁

@jryans
Copy link
Collaborator

jryans commented Feb 11, 2020

This is sort of a breaking change, as the extra argument consumers of the js-sdk might still pass will be ignored.

Right, it seems that way. I added a breaking change label to this PR.

@bwindels bwindels merged commit 6684574 into develop Feb 13, 2020
@t3chguy t3chguy deleted the bwindels/dontpassmethodstoverify branch May 10, 2022 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

remove method parameter in requestVerification(DM)
2 participants