Fix disabled zones - Githubissues

thomasgermain / pymultiMATIC

Python interface with Vaillant multiMATIC

MIT License

56 stars 11 forks source link

Fix disabled zones #37

Closed h4de5 closed 4 years ago

h4de5 commented 4 years ago

Fixes #35

thomasgermain commented 4 years ago

Hi,

thanks for the PR. This one is a bit special, because this is the first one done by somebody else then me :smiley:

In order to make the PR build, the doc has to be updated, It can be done by calling make inside the doc folder. (You will need to activate the venv).

Can you also add a test ?

Thanks

h4de5 commented 4 years ago

ok. tests seem to be ok:

python3.8 -m unittest
......................................................................................................................
----------------------------------------------------------------------
Ran 118 tests in 0.090s

OK

the docs update seems to be pretty big.

h4de5 commented 4 years ago

mmh. ./html-diff.sh does not give me any output after make.

h4de5 commented 4 years ago

ok. if github goes down, then because of me refreshing on that status page...

2 of 3 successfull tests ..

thomasgermain commented 4 years ago

Actually, you have to run make html in order to generate html. I also had to remember how to regenerate the doc :disappointed_relieved:

h4de5 commented 4 years ago

make html removed now one of the jquery files - but left the html untouched. I hope thats ok?

h4de5 commented 4 years ago

is it possible, that the last check is your approval?

thomasgermain commented 4 years ago

Yes I have to approve it :yum:

I was expecting the Travis CI - Branch to run, but I guess it won't because this is a fork.

To me this is ok, I will just ask you to squash commits.

Sorry for the doc issue, I actually didn't find any better scenario to keep the doc up to date (if you have any suggestion, feel free :D)

h4de5 commented 4 years ago

actually I was about to copy that CI part for my own HA integration - so no worries.

btw: I could use some (a lot of) help with config_flow, reproduce_state and some other HA related stuff - so..as soon as sort out that odds and ends here, feel free to drop by 😁

https://community.home-assistant.io/t/vimar-integration-wip/202073

regarding the squash: ~~a new PR?~~ I think you should be able to do this during merging:

thomasgermain commented 4 years ago

Oh yeah, I didn't realize I could do it myself :disappointed_relieved: I'm used to do it via command line :)

Anyway, Thanks for your contribution, I'm gonna merge it and release it.

Feel free to ping me if you need some help for config flow :)