search

LukasGrebe / ha-addons

Addons for Home Assistant
GNU General Public License v3.0
37 stars 42 forks source link

Various changes and improvements #11

Closed tim-devel closed 2 years ago

tim-devel commented 2 years ago

Hi, I have been doing some work in my local branch. All changes are listed in the changelog

LukasGrebe commented 2 years ago

I REALLY appreciate your work! thank you so much.

the conversations might get confusing (as per review I've started, sharing multiple topics) - is there an easy way to split this into multiple pull requests, to keep the conversations focused on a single feature addition or change? - if not, I'm sure we can make this pull request work anyways. I'd just be a bit more sorted otherwise…

tim-devel commented 2 years ago

Hi, I would usually do one change at a time via individual pull requests but I have made quite a few changes during your hiatus.

I can try and split these out but I think I would need to roll back to your fork and add the changes in one by one. ☹️☹️

The change you merged earlier already conflicts with these changes so might not have another choice ☹️

LukasGrebe commented 2 years ago

Dang, sorry about that. this is my first repo that anyone else has made changes and pull requests to. I'll try my very best to make this as easy for you as possible. would you mind taking a look at and replying to some of the comments in the review? https://github.com/LukasGrebe/ha-addons/pull/11/files/e10f440f4db875f15d0251b469ff00434ebfc030

tim-devel commented 2 years ago

Yeah, no problem, but I can't see any comments?

LukasGrebe commented 2 years ago

Yeah, no problem, but I can't see any comments?

hmm odd - I followed these instructions: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/commenting-on-a-pull-request#adding-line-comments-to-a-pull-request - as far as I can tell you should see them within the linked file, or when scrolling through the discussion view of this pull request.

tim-devel commented 2 years ago

Sorry but definitely cant see comments image

tim-devel commented 2 years ago

I have fixed the merge conflict but still can't see your in line comments :-(

tim-devel commented 2 years ago

I have added an in-line comment as a test

tim-devel commented 2 years ago

I think I have answered all your comments. This is ready to merge as long as you are happy with marking wireless adapters as BETA and fixing in a new PR.

I will make sure any changes are broken down into single PR's going forwards

LukasGrebe commented 2 years ago

Done and again, thank you so much for the contribution!