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

Allow omitting some optional fields #91

Closed
wants to merge 3 commits into from

Conversation

auscompgeek
Copy link

Presence, membership, and profile changes have optional fields.
These should be omitted rather than null when unwanted.

Fixes: #79

Presence, membership, and profile changes have optional fields.
These should be omitted rather than null when unwanted.

Fixes: turt2live#79
@turt2live
Copy link
Owner

Many of these are null for specific reasons, largely for server compatibility in off-spec behaviour.

@auscompgeek
Copy link
Author

Hmm. Do you have any documentation for these broken behaviours anywhere?

@turt2live
Copy link
Owner

nope, hence "off-spec" :p

Most of it is synapse, though there's other implementations out there that rely on a literal null value.

@auscompgeek
Copy link
Author

Hmmm. Well, we already have clients that omit the kick/ban reason when unspecified, and we know #79 is a problem with Synapse. If it's just the profile change endpoints that have problems with being omitted, I can change those signatures back I guess.

@turt2live
Copy link
Owner

tbh I'd rather push forward on the spec side than trya nd fix/document all the problems in the library.

src/MatrixClient.ts Outdated Show resolved Hide resolved
@Half-Shot Half-Shot mentioned this pull request Feb 20, 2021
@turt2live turt2live closed this Mar 18, 2021
@turt2live turt2live deleted the branch turt2live:master March 18, 2021 18:46
@turt2live turt2live reopened this Mar 18, 2021
@turt2live turt2live changed the base branch from develop to master March 18, 2021 18:47
@turt2live
Copy link
Owner

closing this per spec concerns

@turt2live turt2live closed this May 2, 2022
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.

setPresenceStatus status_msg defaults to null
3 participants