Skip to content

Conversation

Sartoric
Copy link
Contributor

@Sartoric Sartoric commented Nov 3, 2024

Add support for cover position for the Italian language

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 @Sartoric

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 November 3, 2024 18:38
@home-assistant
Copy link

home-assistant bot commented Nov 3, 2024

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.

@auanasgheps
Copy link
Contributor

Looks good, thanks!

@auanasgheps
Copy link
Contributor

It's marked as draft: are you still working on this?

@Sartoric
Copy link
Contributor Author

Sartoric commented Nov 6, 2024

It's marked as draft

Oh, it wasn't intentional.

are you still working on this?

No, I'm done

Well, in fact I'm just updating some sentences to use min/max position.
I'll send a new PR when done

- updated sentences with expansion rules
- added support for min/med/max position (list and sentences)
- minor fixes
- minor fixes to expansion rules and blank spaces
@Sartoric Sartoric marked this pull request as ready for review November 7, 2024 17:53
@balloob
Copy link
Contributor

balloob commented Nov 13, 2024

This PR is showing issues that are caught by pre-commit. Pre-commit should run automatically when making a commit to fix files.

@balloob
Copy link
Contributor

balloob commented Nov 13, 2024

@auanasgheps @Sartoric I fixed the file formatting, it should be good to go now.

When looking at the intent dashboard, I noticed that Italian only needs HassListAddItem intent coverage to be considered usable. Would you be able to add this?

@balloob
Copy link
Contributor

balloob commented Nov 13, 2024

I have created a template generator to get LLMs to help with intent coverage in #2528. It created these files to kickstart Italian coverage of HassListAddItem: https://chatgpt.com/share/6734314d-3360-8002-a121-dbd2e80e4e10

@auanasgheps
Copy link
Contributor

Thanks @balloob, I'll look into your suggestion and verify it!

@Sartoric
Copy link
Contributor Author

@auanasgheps
I can work on it if you need help

@Sartoric
Copy link
Contributor Author

Sartoric commented Nov 13, 2024

I'm trying to add the HassListAddItem
but the test return
AssertionError: Recognition failed for 'aggiungi mele alla mia lista trader joes'

It should be a correct case, so I'm probably missing something stupid here :)
Could it be related to something else ?

@Sartoric
Copy link
Contributor Author

It was indeed a stupid thing :)
I've sent a new PR

@auanasgheps
Copy link
Contributor

Approving this one too. Thanks for your work.

@auanasgheps auanasgheps merged commit 5081b82 into OHF-Voice:main Nov 16, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants