-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Code cleanup and improved typings #3072
Code cleanup and improved typings #3072
Conversation
Pull Request Test Coverage Report for Build 4645292148
💛 - Coveralls |
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.
Thanks improving some of the typings of the project 🚀
I had two minor points. Apart from that, this PR looks good to me 👍
@@ -120,7 +126,7 @@ export interface StaticPlatformPlugin extends PlatformPlugin { | |||
* | |||
* @param {(foundAccessories: AccessoryPlugin[]) => void} callback | |||
*/ | |||
accessories(callback: (foundAccessories: AccessoryPlugin[]) => void): void; | |||
accessories(callback: (foundAccessories: Array<AccessoryPlugin & AccessoryConfig>) => void): void; |
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.
I think this typing doesn't really represent reality (and might even break current implementations?).
The weird thing is, that we really only require the name: string
property (and optionally check for the uuid_base?: string
property). Using AccessoryConfig
though will add the accessory: AccessoryName | AccessoryIdentifier
requirement and the optional _bridge?: BridgeConfiguration
property which both are never used.
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.
// @ts-ignore | ||
const uuidBase: string | undefined = accessoryInstance.uuid_base; // optional base uuid | ||
|
||
log.info("Initializing platform accessory '%s'...", accessoryName); |
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.
Please don't remove this log statement.
@@ -120,7 +126,7 @@ export interface StaticPlatformPlugin extends PlatformPlugin { | |||
* | |||
* @param {(foundAccessories: AccessoryPlugin[]) => void} callback | |||
*/ | |||
accessories(callback: (foundAccessories: AccessoryPlugin[]) => void): void; |
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.
ericanderson:cleanup-ts-ignore
Co-authored-by: Andi <mail@anderl-bauer.de>
Hi all, just popping into get a status update on this! @ericanderson and @Supereg any status update on this:? cc @donavanbecker for context Cheers, Ben! |
@ericanderson, please reopen direct towards |
This is a code cleanup.