Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[DE] add support for person_HassGetState #1657

Merged
merged 10 commits into from
Dec 12, 2023
Merged

[DE] add support for person_HassGetState #1657

merged 10 commits into from
Dec 12, 2023

Conversation

steffenrapp
Copy link
Contributor

@steffenrapp steffenrapp commented Nov 19, 2023 •
edited
Loading

Can get some more fine tuning but it's a start.

@steffenrapp steffenrapp marked this pull request as ready for review November 19, 2023 16:21
mib1185
mib1185 requested changes Dec 9, 2023
View reviewed changes
Copy link
Contributor

@mib1185 mib1185 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @steffenrapp
as always, thanks for contribution 👍
I think we should adopt the Dutch approach here

responses/de/HassGetState.yaml Outdated Show resolved Hide resolved
@home-assistant home-assistant bot marked this pull request as draft December 9, 2023 14:39
@home-assistant
Copy link

home-assistant bot commented Dec 9, 2023

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

mib1185
mib1185 requested changes Dec 10, 2023
View reviewed changes
tests/de/person_HassGetState.yaml Outdated Show resolved Hide resolved
tests/de/person_HassGetState.yaml Outdated Show resolved Hide resolved
@mib1185 mib1185 changed the title [DE] person_HassGetState [DE] add support for person_HassGetState Dec 10, 2023
@mib1185 mib1185 marked this pull request as ready for review December 10, 2023 00:20
@mib1185 mib1185 enabled auto-merge (squash) December 10, 2023 00:26
mib1185
mib1185 reviewed Dec 10, 2023
View reviewed changes
tests/de/_fixtures.yaml Outdated Show resolved Hide resolved
@steffenrapp
Copy link
Contributor Author

@mib1185 thanks for the review and the contributions. I'm working on fixing the errors. Have to rebuild some of the logic.

auto-merge was automatically disabled December 10, 2023 10:41

Head branch was pushed to by a user without write access

@steffenrapp
Copy link
Contributor Author

steffenrapp commented Dec 10, 2023 •
edited
Loading

@mib1185 I adapted the logic and added more tests. Hope it's fine, maybe I can do a little bit more refinement in a separate PR afterwards. Also I want to use the new expansion rule <wieviel> everywhere else where it fits.

mib1185
mib1185 requested changes Dec 10, 2023
View reviewed changes
Copy link
Contributor

@mib1185 mib1185 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @steffenrapp there are some comments from my side, thx 👍

Also I want to use the new expansion rule everywhere else where it fits.

great idea - let's keep this for a later PR 👍

sentences/de/person_HassGetState.yaml Show resolved Hide resolved
sentences/de/person_HassGetState.yaml Outdated Show resolved Hide resolved
responses/de/HassGetState.yaml Show resolved Hide resolved
@home-assistant home-assistant bot marked this pull request as draft December 10, 2023 12:42
@steffenrapp steffenrapp marked this pull request as ready for review December 10, 2023 13:05
@mib1185 mib1185 marked this pull request as draft December 10, 2023 13:57
@steffenrapp steffenrapp marked this pull request as ready for review December 10, 2023 15:05
@mib1185 mib1185 changed the base branch from main to untranslated-state December 12, 2023 21:15
@mib1185
Copy link
Contributor

mib1185 commented Dec 12, 2023

Hi @steffenrapp
based on #1768 (reply in thread) this branch has been rebased on untranslated-state and is going to be merged there.

@mib1185 mib1185 merged commit cd9a8ee into home-assistant:untranslated-state Dec 12, 2023
2 checks passed
@steffenrapp steffenrapp deleted the de-person_HassGetState branch December 15, 2023 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mib1185 mib1185 mib1185 approved these changes

@easterapps easterapps Awaiting requested review from easterapps easterapps is a code owner

Assignees
No one assigned
Labels
cla-signed lang: de
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@steffenrapp @mib1185