homebridge / HAP-NodeJS

Node.js implementation of the HomeKit Accessory Protocol (HAP)
Apache License 2.0
2.69k stars 630 forks source link

Bridged core and core cleanup #1048

Closed Shaquu closed 4 months ago

Shaquu commented 4 months ago

:recycle: Current situation

Describe the current situation. Explain current problems, if there are any. Be as descriptive as possible (e.g., including examples or code snippets).

  1. Current code holds incorrect deprecation date, 2022 seems like a little outdated
  2. There are some typos
  3. There are unhandled promises :)
  4. Forbidden char in Sprinkler example!

:bulb: Proposed solution

Describe the proposed solution and changes. How does it affect the project? How does it affect the internal structure (e.g., refactorings)?

  1. Current code holds incorrect deprecation date, 2022 seems like a little outdated
  2. I did some clean-up regarding deprecation warning
  3. I did handled promises!
  4. Removed forbidden chars
coveralls commented 4 months ago

Pull Request Test Coverage Report for Build 9669593296

Details


Changes Missing Coverage Covered Lines Changed/Added Lines %
src/BridgedCore.ts 0 4 0.0%
src/Core.ts 0 4 0.0%
<!-- Total: 0 8 0.0% -->
Totals Coverage Status
Change from base Build 9655031976: 0.005%
Covered Lines: 7414
Relevant Lines: 10615

💛 - Coveralls
coveralls commented 4 months ago

Pull Request Test Coverage Report for Build 9669634941

Details


Changes Missing Coverage Covered Lines Changed/Added Lines %
src/BridgedCore.ts 0 4 0.0%
src/Core.ts 0 4 0.0%
<!-- Total: 0 8 0.0% -->
Totals Coverage Status
Change from base Build 9655031976: 0.005%
Covered Lines: 7414
Relevant Lines: 10615

💛 - Coveralls
Shaquu commented 4 months ago

couple of comments.

i will always veer towards asking if any of the changes you have made could potentially breaking in any way

One potential break. I removed 💦 from Accessory and Service. It was used in AccessoryInformation Characteristic. But from what I see it was not used to generate UUID.

coveralls commented 4 months ago

Pull Request Test Coverage Report for Build 9669903686

Details


Changes Missing Coverage Covered Lines Changed/Added Lines %
src/BridgedCore.ts 0 4 0.0%
src/Core.ts 0 4 0.0%
<!-- Total: 0 8 0.0% -->
Files with Coverage Reduction New Missed Lines %
src/lib/Accessory.ts 3 89.29%
<!-- Total: 3 -->
Totals Coverage Status
Change from base Build 9655031976: -0.07%
Covered Lines: 7411
Relevant Lines: 10615

💛 - Coveralls
coveralls commented 4 months ago

Pull Request Test Coverage Report for Build 9676782347

Details


Changes Missing Coverage Covered Lines Changed/Added Lines %
src/BridgedCore.ts 0 4 0.0%
src/Core.ts 0 4 0.0%
<!-- Total: 0 8 0.0% -->
Totals Coverage Status
Change from base Build 9673726680: 0.005%
Covered Lines: 7411
Relevant Lines: 10615

💛 - Coveralls
Shaquu commented 4 months ago

Ready to be merged?

coveralls commented 4 months ago

Pull Request Test Coverage Report for Build 9711844463

Details


Changes Missing Coverage Covered Lines Changed/Added Lines %
src/BridgedCore.ts 0 4 0.0%
src/Core.ts 0 4 0.0%
<!-- Total: 0 8 0.0% -->
Totals Coverage Status
Change from base Build 9690647033: 0.08%
Covered Lines: 7414
Relevant Lines: 10615

💛 - Coveralls