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

Test moving aliases on room upgrades #517

Merged
merged 4 commits into from
Oct 31, 2018
Merged

Conversation

richvdh
Copy link
Member

@richvdh richvdh commented Oct 27, 2018

Builds on #513. Tests for matrix-org/synapse#4101.

@richvdh richvdh requested a review from a team October 27, 2018 01:12
Copy link
Member

@erikjohnston erikjohnston left a comment

Choose a reason for hiding this comment

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

Looks good apart from the boiler plate code to wait for canonical alias to come down sync. It'd be good to find a way to reduce that in both this PR and the other

@@ -51,6 +52,7 @@ sub upgrade_room {
new_version => $new_version,
},
)->then( sub {

Copy link
Member

Choose a reason for hiding this comment

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

(Did you mean to add these new lines?)

}
return 1;
},
);
Copy link
Member

Choose a reason for hiding this comment

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

(This feels sad the amount of boiler plate code here)

@richvdh richvdh requested a review from erikjohnston October 30, 2018 16:26
matrix_get_room_state(
$creator, $room_id,
type=>'m.room.aliases', state_key=>$server_name,
);
Copy link
Member

Choose a reason for hiding this comment

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

Any reason for using matrix_get_room_state rather than just syncing?

Copy link
Member Author

Choose a reason for hiding this comment

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

I decided it was going to be a pain to find the right events in the sync. Could change it though.

Copy link
Member

Choose a reason for hiding this comment

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

Nah, its fine

@richvdh richvdh force-pushed the rav/aliases_for_upgrades branch from 3c084da to 6a6653a Compare October 31, 2018 17:53
@richvdh
Copy link
Member Author

richvdh commented Oct 31, 2018

sigh. retest this please.

@richvdh richvdh merged commit e206063 into develop Oct 31, 2018
@richvdh richvdh deleted the rav/aliases_for_upgrades branch October 31, 2018 19:47
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.

2 participants