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.

Sign up for GitHub

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

Jump to bottom

Add top level filters for filtering by room id. #246

Merged
merged 5 commits into from
Dec 23, 2015
Merged

Add top level filters for filtering by room id. #246

merged 5 commits into from
Dec 23, 2015

Conversation

NegativeMjark
Copy link
Contributor

Based on #245.

@NegativeMjark NegativeMjark mentioned this pull request Dec 21, 2015
Merged
kegsay
kegsay reviewed Dec 22, 2015
View reviewed changes
api/client-server/definitions/sync_filter.yaml
room:
properties:
not_rooms:
description: A list of room IDs to exclude. If this list is absent then no rooms
are excluded. A matching room will be excluded even if it is listed in the 'rooms'
Copy link
Member

Choose a reason for hiding this comment

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

Backticks on 'rooms' please.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed 9b4a37f

@kegsay
Copy link
Member

kegsay commented Dec 22, 2015

We're doing this for the purpose of guest access. We should probably mention that these filters will determine the rooms, or were you planning on doing that in another PR? It feels a little ming to be putting the global filter definition at the same level as the ephemeral and account_data keys. Perhaps we should use a new keyword like global?

NegativeMjark added a commit to matrix-org/synapse that referenced this pull request Dec 22, 2015
@NegativeMjark
Add top level filters for filtering by room id
b9b4466 
Documented by matrix-org/matrix-spec-proposals#246
NegativeMjark added a commit to matrix-org/synapse that referenced this pull request Dec 22, 2015
@NegativeMjark
Merge pull request #454 from matrix-org/markjh/room_filtering
e525b46 
Add top level filters for filtering by room id

Documented by matrix-org/matrix-spec-proposals#246
@NegativeMjark
Copy link
Contributor Author

I was going to leave it to another PR to explain how they were used in guest access.

@kegsay
Copy link
Member

kegsay commented Dec 22, 2015

Okay. Thoughts about shoving this to a global section?

@NegativeMjark
Copy link
Contributor Author

I thought using the hierarchical position of the filters to indicate which parts of the response they applied to made it clearer than adding a "global" section. I would regret that if we ever came to add a conflicting key name.

@kegsay
Copy link
Member

kegsay commented Dec 23, 2015

Okay, that seems like a good enough reason to keep them where they are. If it turns out it's a mistake, we can always change it :) LGTM

NegativeMjark added a commit that referenced this pull request Dec 23, 2015
@NegativeMjark
Merge pull request #246 from matrix-org/markjh/room_filter
f648360 
Add top level filters for filtering by room id.
@NegativeMjark NegativeMjark merged commit f648360 into master Dec 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@kegsay kegsay

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@NegativeMjark @kegsay