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

Port Bridge to Typescript #213

Merged
merged 10 commits into from
Sep 1, 2020
Merged

Port Bridge to Typescript #213

merged 10 commits into from
Sep 1, 2020

Conversation

Half-Shot
Copy link
Contributor

Depends on the remaining TS PRs, so #209 and #210.

This PR ports the last file to Typescript and thus completes the Typescript project. After this, we can safely merge #199

@Half-Shot Half-Shot requested a review from a team August 24, 2020 15:35
@jaller94
Copy link
Contributor

Waiting for you to fix the tests.

@jaller94 jaller94 removed the request for review from a team August 24, 2020 15:37
onInvite(ev) {
if (!this._waitingForInvite.has(ev.room_id)) {
// eslint-disable-next-line camelcase
public async onInvite(ev: {room_id: string}) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this returning a boolean and what's the information this boolean carries?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Have improved the JSDoc for the whole class.

@Half-Shot Half-Shot requested a review from jaller94 September 1, 2020 09:15
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.

Looks to be ok.
I found it especially though to compare bridge.js and bridge.ts.

@Half-Shot Half-Shot merged commit c603707 into develop Sep 1, 2020
@jaller94 jaller94 deleted the hs/port-bridge branch September 1, 2020 16:08
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