Closed ollo69 closed 2 years ago
Thank you very much for this. I'll look through it over the next couple of days. I'm sure it is right, but it also helps me to educate myself on how things should be done properly!
Hiya, I've made a minor change (and Sourcery may suggest more but I'll deal with that after). You passed HASS through when making the call to _async_get_device_info. Is there a reason for doing so? It seems to work fine with the HASS available in self.hass.
Basically because I removed inheritance from Entity
in SkyQEntity
class, self.hass
is not anymore attribute of this class but is inherit from the derived class. So this basically work, but that class in this way is not autonomous because that method do not work if that class is derived by another that not provide that attribute. Probably if you want to use self.hass
should be better to restore inheritance from base class Entity
. Generally speaking a class should never use method of attributes of the classes that it is derived from, only the ones that it derived from or declared in the class itself.
OK, I'll revert the changes I made.
✅ Merging this PR will increase code quality in the affected files by 0.50%.
Quality metrics | Before | After | Change |
---|---|---|---|
Complexity | 3.09 ⭐ | 3.00 ⭐ | -0.09 👍 |
Method Length | 34.06 ⭐ | 33.07 ⭐ | -0.99 👍 |
Working memory | 5.46 ⭐ | 5.47 ⭐ | 0.01 👎 |
Quality | 82.64% ⭐ | 83.14% ⭐ | 0.50% 👍 |
Other metrics | Before | After | Change |
---|---|---|---|
Lines | 639 | 648 | 9 |
Changed files | Quality Before | Quality After | Quality Change |
---|---|---|---|
custom_components/skyq/entity.py | 83.40% ⭐ | 85.46% ⭐ | 2.06% 👍 |
custom_components/skyq/media_player.py | 80.99% ⭐ | 81.55% ⭐ | 0.56% 👍 |
custom_components/skyq/sensor.py | 93.20% ⭐ | 93.20% ⭐ | 0.00% |
Here are some functions in these files that still need a tune-up:
File | Function | Complexity | Length | Working Memory | Quality | Recommendation |
---|---|---|---|---|---|---|
custom_components/skyq/media_player.py | SkyQDevice.__init__ | 3 ⭐ | 133 😞 | 6 ⭐ | 70.90% 🙂 | Try splitting into smaller methods |
The emojis denote the absolute quality of the code:
The 👍 and 👎 indicate whether the quality has improved or gotten worse with this pull request.
Please see our documentation here for details on how these metrics are calculated.
We are actively working on this report - lots more documentation and extra metrics to come!
Help us improve this quality report!
There is a little bug introduced with the new disk usage sensor that could cause the name of the device not properly set. Device could assume the name of the sensor (suffixed with "Used Storage") in case that the sensor is initialized before the
media_player
entity. Because it seems logic to me to move some common variable inside the common classSkyQEntity
that are used there to determinate device information, I moved initialization there, hoping that I correctly understand the means of this class.