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

[PT] Improve cover intent #2101

Merged
merged 3 commits into from
Mar 25, 2024
Merged

[PT] Improve cover intent #2101

merged 3 commits into from
Mar 25, 2024

Conversation

marcocunha
Copy link
Contributor

@marcocunha marcocunha commented Mar 21, 2024 •
edited
Loading

Add 'Estores'

home-assistant[bot]
home-assistant bot previously requested changes Mar 21, 2024
View reviewed changes
Copy link

@home-assistant home-assistant bot left a comment

Choose a reason for hiding this comment

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

Hi @marcocunha

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

@home-assistant home-assistant bot marked this pull request as draft March 21, 2024 17:30
@home-assistant
Copy link

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.

marcocunha
marcocunha commented Mar 21, 2024
View reviewed changes
Copy link
Contributor Author

@marcocunha marcocunha 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.

@joaorgoncalves I added a test case, but it fails, so I added it as a comment. Can you please check what might be the reason?

@abmantis
Copy link
Contributor

abmantis commented Mar 23, 2024 •
edited
Loading

Thanks for the improvements!
Unfortunately the cover lists had some issues, since they cannot have duplicated "in" values for the same "out".
While testing this out, I've ended up updated them to match the ones that are currently displayed by HA UI in PT, and fixing your test. You can check the changes here: 100cb48

I don't have permissions to apply it to your branch.

@marcocunha
Copy link
Contributor Author

That's great @abmantis . Just updated this pull request.

@marcocunha marcocunha marked this pull request as ready for review March 24, 2024 18:59
@abmantis abmantis enabled auto-merge (squash) March 25, 2024 01:03
@abmantis abmantis merged commit 283a15a into home-assistant:main Mar 25, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abmantis abmantis abmantis approved these changes

@home-assistant home-assistant home-assistant left review comments

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

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

Successfully merging this pull request may close these issues.
2 participants
@marcocunha @abmantis