-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Migrated modules/rtc/MockClasses
#2657
base: master
Are you sure you want to change the base?
Conversation
@saghul, could you please review this? It introduces some breaking changes, so I’d appreciate it if you could verify that everything is working correctly and nothing is inadvertently broken. |
Hi, thanks for your contribution! |
this.height = height; | ||
} | ||
|
||
/** | ||
* Returns height. | ||
* @returns {number} | ||
*/ | ||
getSettings() { | ||
getSettings(): { height: number } { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this return type is a number, but you are returning an object
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the previous implementation of getSettings()
already returned an object, and my TypeScript migration keeps the same structure with proper typing. and for this i also create getter getHeight()
as it is also returning the same thing as a number
fixes #2637
Key Changes