/synapse

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Don't require alias in public room list. #664

Merged
merged 6 commits into from Mar 23, 2016

Conversation

Reviewers
No reviews
Assignees

@NegativeMjark NegativeMjark

Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@erikjohnston @NegativeMjark @oddvar
@erikjohnston
Owner

erikjohnston commented Mar 23, 2016

Rooms now no longer require an alias to be published.

Also, changes the way we pull out state of each room to not require fetching all state events.
@erikjohnston
Don't require alias in public room list. 
Rooms now no longer require an alias to be published.

Also, changes the way we pull out state of each room to not require
fetching all state events.

All checks have passed

8 successful checks
@matrixbot
Flake8 + Packaging (Commit) — Build #193 origin/erikj/public_room_list succeeded in 25 sec
Details
@matrixbot
Flake8 + Packaging (Merged PR) — Build finished.
Details
@matrixbot
Sytest Postgres (Commit) — Build #196 origin/erikj/public_room_list succeeded in 5 min 41 sec
Details
@matrixbot
Sytest Postgres (Merged PR) — Build finished.
Details
@matrixbot
Sytest SQLite (Commit) — Build #197 origin/erikj/public_room_list succeeded in 3 min 23 sec
Details
@matrixbot
Sytest SQLite (Merged PR) — Build finished.
Details
@matrixbot
Unit Tests (Commit) — Build #238 origin/erikj/public_room_list succeeded in 1 min 5 sec
Details
@matrixbot
Unit Tests (Merged PR) — Build finished.
Details
34473a9

@erikjohnston erikjohnston assigned NegativeMjark Mar 23, 2016

@oddvar oddvar added the in progress label Mar 23, 2016

@NegativeMjark NegativeMjark commented on an outdated diff Mar 23, 2016

View full outdated diff
synapse/handlers/room.py
- name_event = state.get((EventTypes.Name, ""), None)
+ name_event = yield get_state(EventTypes.Name, "")
@NegativeMjark

NegativeMjark Mar 23, 2016

Contributor

I guess this is so we don't have to pull in the entire room state for each room? Might be worth adding a comment explaining why we've pick this particular performance trade off.

@NegativeMjark NegativeMjark and 1 other commented on an outdated diff Mar 23, 2016

View full outdated diff
synapse/handlers/room.py
- result = {"aliases": aliases, "room_id": room_id}
+ result = {"room_id": room_id}
+ if aliases:
+ result["aliases"] = aliases
@NegativeMjark

NegativeMjark Mar 23, 2016

Contributor

I wonder if we should give an empty list rather than omit the key.

@erikjohnston

erikjohnston Mar 23, 2016

Owner

Hmm, yeah, maybe. I just copied how the other keys worked (i.e. omitting the key)
@erikjohnston
Owner

erikjohnston commented Mar 23, 2016

retest this please

erikjohnston added some commits Mar 23, 2016

@erikjohnston
Comment
0677fc1
@erikjohnston
Ensure published rooms have public join rules
b2802a1
@erikjohnston
Ensure all old public rooms have aliases
84afeb4
@erikjohnston
SQLite and postgres doesn't share a true literal

All checks have passed

8 successful checks
@matrixbot
Flake8 + Packaging (Commit) — Build #198 origin/erikj/public_room_list succeeded in 28 sec
Details
@matrixbot
Flake8 + Packaging (Merged PR) — Build finished.
Details
@matrixbot
Sytest Postgres (Commit) — Build #201 origin/erikj/public_room_list succeeded in 5 min 25 sec
Details
@matrixbot
Sytest Postgres (Merged PR) — Build finished.
Details
@matrixbot
Sytest SQLite (Commit) — Build #202 origin/erikj/public_room_list succeeded in 4 min 23 sec
Details
@matrixbot
Sytest SQLite (Merged PR) — Build finished.
Details
@matrixbot
Unit Tests (Commit) — Build #243 origin/erikj/public_room_list succeeded in 1 min 12 sec
Details
@matrixbot
Unit Tests (Merged PR) — Build finished.
Details
0c1a27b
@erikjohnston
Comment about weird SQL

All checks have passed

8 successful checks
@matrixbot
Flake8 + Packaging (Commit) — Build #199 origin/erikj/public_room_list succeeded in 28 sec
Details
@matrixbot
Flake8 + Packaging (Merged PR) — Build finished.
Details
@matrixbot
Sytest Postgres (Commit) — Build #202 origin/erikj/public_room_list succeeded in 5 min 18 sec
Details
@matrixbot
Sytest Postgres (Merged PR) — Build finished.
Details
@matrixbot
Sytest SQLite (Commit) — Build #203 origin/erikj/public_room_list succeeded in 4 min 39 sec
Details
@matrixbot
Sytest SQLite (Merged PR) — Build finished.
Details
@matrixbot
Unit Tests (Commit) — Build #244 origin/erikj/public_room_list succeeded in 1 min 15 sec
Details
@matrixbot
Unit Tests (Merged PR) — Build finished.
Details
b275765
@erikjohnston
Owner

erikjohnston commented Mar 23, 2016

@NegativeMjark PTAL

I'm afraid I added some SQL and a check for JoinRules = Public
@NegativeMjark
Contributor

NegativeMjark commented Mar 23, 2016

LGTM

@erikjohnston erikjohnston merged commit fbdeb17 into develop Mar 23, 2016
8 checks passed

8 checks passed

Flake8 + Packaging (Commit) Build #199 origin/erikj/public_room_list succeeded in 28 sec
Details
Flake8 + Packaging (Merged PR) Build finished.
Details
Sytest Postgres (Commit) Build #202 origin/erikj/public_room_list succeeded in 5 min 18 sec
Details
Sytest Postgres (Merged PR) Build finished.
Details
Sytest SQLite (Commit) Build #203 origin/erikj/public_room_list succeeded in 4 min 39 sec
Details
Sytest SQLite (Merged PR) Build finished.
Details
Unit Tests (Commit) Build #244 origin/erikj/public_room_list succeeded in 1 min 15 sec
Details
Unit Tests (Merged PR) Build finished.
Details

@oddvar oddvar removed the in progress label Mar 23, 2016

@dbkr dbkr referenced this pull request in vector-im/riot-web Mar 24, 2016

Closed

Room settings: need a warning when setting a room public without address #767

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment