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

chore: update packages #295

Merged
merged 5 commits into from
Jan 15, 2021
Merged

chore: update packages #295

merged 5 commits into from
Jan 15, 2021

Conversation

Half-Shot
Copy link
Contributor

No description provided.

@Half-Shot Half-Shot requested a review from a team January 14, 2021 12:48
@@ -831,7 +831,8 @@ export class Intent {
try {
await this.ensureRegistered();
if (dontJoin) {
deferredPromise.resolve();
// XXX: Should we return the passed in parameter if we didn't join, or empty?
Copy link
Contributor

Choose a reason for hiding this comment

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

I think returning the roomIdOrAlias is ok as it imitates the success state after a join that's managed by this lib.
This is just for the public function join, right? There doesn't seem to be another use of the return value.

public async join(roomIdOrAlias: string, viaServers?: string[]): Promise<string> {

And we're changing this from void to string because that's what the public function join promises, correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup. Join should always return the room_id on success.

Copy link
Contributor

@jaller94 jaller94 left a comment

Choose a reason for hiding this comment

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

Nice upgrade to TypeScript, as well as other dependencies.

@Half-Shot Half-Shot merged commit 7e0bf8e into develop Jan 15, 2021
@jaller94 jaller94 deleted the hs/package-updates branch January 15, 2021 12:16
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