Skip to content

Conversation

@Androbin
Copy link
Contributor

Fixes #159
@Matrixcoffee /cc
@non-Jedi /cc

@non-Jedi
Copy link
Collaborator

Signoff please and fix following PEP8 issues:

  • matrix_client/checks.py:8:1: E302 expected 2 blank lines, found 1
  • matrix_client/user.py:3:1: E302 expected 2 blank lines, found 1

Signed-off-by: Robin Richtsfeld <robin.richtsfeld@gmail.com>
@Androbin
Copy link
Contributor Author

Fixed PEP8, rebased and signed off

@non-Jedi
Copy link
Collaborator

LGTM. Appreciate this. :)

@non-Jedi non-Jedi merged commit fdfe8a0 into matrix-org:master May 15, 2018
@Androbin Androbin deleted the checks branch May 15, 2018 18:09
@Androbin
Copy link
Contributor Author

Where else could potentially "dirty" room_id and user_id values be passed?
Should we add this to deprecated client.set_user_id or wait for it's removal?

@non-Jedi
Copy link
Collaborator

non-Jedi commented May 15, 2018

Not sure where else, and I'm ambivalent on adding it/not adding it to deprecated methods.

@ghost
Copy link

ghost commented May 23, 2018

Great stuff.

Just one remark: Although room IDs should be fitted with a domain part on creation, those are only informational, and room IDs should be treated as opaque strings anywhere else. Since the domain part isn't actually used by anything, the utility of checking for it seems marginal at best, and it may cause trouble down the road.

@Androbin
Copy link
Contributor Author

How about an additional defaulted flag?
def check_room_id(room_id, require_domain=False):
We would then pass require_domain=True on creation.

@non-Jedi
Copy link
Collaborator

While room IDs may one day be truly opaque and should in general be treated as such by clients, the spec currently says that room ids must have a domain part, so I think leaving the check as-is is reasonable until the spec is changed.

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.

2 participants