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

Update type definition #3125

Merged
merged 5 commits into from
Jan 11, 2023
Merged

Update type definition #3125

merged 5 commits into from
Jan 11, 2023

Conversation

jerry4718
Copy link
Contributor

No description provided.

@januscla
Copy link

Thanks for your contribution, @jerry4718! Please make sure you sign our CLA, as it's a required step before we can merge this.

@lminiero
Copy link
Member

There's a lot of changes here. Can you please elaborate on what led you to come up with these changes, and explain them? Thanks!

@jerry4718
Copy link
Contributor Author

jerry4718 commented Dec 14, 2022 via email

@lminiero
Copy link
Member

Pinging @atoppi as this is beyond my poor JS skills 🤭

Copy link
Member

@atoppi atoppi left a comment

Choose a reason for hiding this comment

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

Thanks for the effort!
I've left some comments.

npm/janus.d.ts Outdated Show resolved Hide resolved
npm/janus.d.ts Outdated Show resolved Hide resolved
npm/janus.d.ts Outdated Show resolved Hide resolved
npm/janus.d.ts Outdated Show resolved Hide resolved
npm/janus.d.ts Outdated Show resolved Hide resolved
npm/janus.d.ts Outdated Show resolved Hide resolved
npm/janus.d.ts Outdated Show resolved Hide resolved
npm/janus.d.ts Outdated Show resolved Hide resolved
npm/janus.d.ts Outdated Show resolved Hide resolved
npm/janus.d.ts Outdated Show resolved Hide resolved
@jerry4718
Copy link
Contributor Author

Thanks for the effort! I've left some comments.

I am really sorry that I have so many negligence, then I will address these points in detail next, thanks for the review

npm/janus.d.ts Outdated Show resolved Hide resolved
npm/janus.d.ts Outdated Show resolved Hide resolved
npm/janus.d.ts Outdated Show resolved Hide resolved
npm/janus.d.ts Outdated Show resolved Hide resolved
npm/janus.d.ts Outdated Show resolved Hide resolved
npm/janus.d.ts Outdated Show resolved Hide resolved
npm/janus.d.ts Show resolved Hide resolved
Copy link
Member

@atoppi atoppi left a comment

Choose a reason for hiding this comment

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

now it looks good !

@lminiero
Copy link
Member

Thanks, merging then!

@lminiero lminiero merged commit 5bfc088 into meetecho:master Jan 11, 2023
@Pratikh
Copy link

Pratikh commented Jan 23, 2023

Hi

We are using Janus in our project with TS so just wanted to know how can use type definition which you defined?

Janus is awesome Thanks @lminiero

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.

5 participants