search

finity69x2 / nws_alerts

An updated version of the nws_alerts custom integration for Home Assistant
86 stars 29 forks source link

fix: attempt to import yaml config #16

Closed firstof9 closed 3 years ago

firstof9 commented 3 years ago
codecov-commenter commented 3 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (dev@6e650cb). Click here to learn what that means. The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##             dev      #16   +/-   ##
======================================
  Coverage       ?   91.26%           
======================================
  Files          ?        4           
  Lines          ?      309           
  Branches       ?        0           
======================================
  Hits           ?      282           
  Misses         ?       27           
  Partials       ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 6e650cb...e01bb77. Read the comment docs.

firstof9 commented 3 years ago

91% is good on a first go 🤖

finity69x2 commented 3 years ago

You are way beyond my level of knowledge on this process at this point.

what is my next step?

merge this PR into the dev branch I assume? Does that also bring in all of the "test" files as well?

Is there anything different at that point that needs to be done in the HA instance to configure it via yaml?

My biggest concern is that, since I have no clue what is going on with most of the code now, I won't be able to maintain it in the future if things change or issues arise. That was still possible before but it's way more complicated now and I think therefore more likely in the future.

Don't get me wrong, I do appreciate the help making it better but I just want to make sure it doesn't get too complex and it can still be useful in the future.

firstof9 commented 3 years ago

merge this PR into the dev branch I assume?

yup

Does that also bring in all of the "test" files as well?

yup

Is there anything different at that point that needs to be done in the HA instance to configure it via yaml?

shouldn't be, should be exactly the same, I'm just trying to get it to import the yaml config properly with the new DataCoordinator

My biggest concern is that, since I have no clue what is going on with most of the code now, I won't be able to maintain it in the future if things change or issues arise. That was still possible before but it's way more complicated now and I think therefore more likely in the future.

All the functions are the exact same, they're just moved into the __init__.py file rather than the sensor.py file. The only difference is how that data get's polled really.

The nice thing about the tests are that we can simulate those test alerts and see how the output looks like and potentially handle them nicer if you want.

If you need anything explained just hit me up.