Skip to content

[FR] Add "minuterie" french translation for timer #3149

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.

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

Merged
merged 2 commits into from
Apr 30, 2025

Conversation

jbblanchet
Copy link
Contributor

Adding "minuterie" option in addition to "minuteur" for timer operations to support French Canadian idioms.

See the Office québécois de la langue française definition of "minuterie" as a translation of "timer".

Happy to add more tests as needed. I tried to keep the PR short.

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

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

expansion_rules:
appele: "(appelé|nommé|surnommé|pour [<le>])"
appele: "(appelé[e]|[sur]nommé[e]|pour [<le>])"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I considered adding this to the _common.yaml file, but wasn't sure if it was appropriate or not. Happy to make the change if needed.

Copy link
Contributor

@jlpouffier jlpouffier Apr 30, 2025

Choose a reason for hiding this comment

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

Let's avoid complexifying expansion rules. Even if what you did is more compact, it's more complex to read for contributors.
Golden rule for us is to keep the intent repo as legible as possible.

Copy link
Contributor Author

@jbblanchet jbblanchet Apr 30, 2025

Choose a reason for hiding this comment

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

Yeah, I might have gotten a little too cute there. I mainly did it because they share the same root, just with and without the prefix, but I see you point. I'll change them back.

FYI I didn't revert the full line, because I still needed the [e] to support the feminine minuterie, but I did revert the [sur]

@jbblanchet jbblanchet marked this pull request as ready for review April 26, 2025 17:30
Copy link
Contributor

@jlpouffier jlpouffier left a comment

Choose a reason for hiding this comment

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

Thx a bunch for the contribution.
It looks good, I provided only one feedback (But copied it everywhere it's relevant)

Additional question: Are you located in Canada? We're looking for contributors in Canada because we're afraid to leave you folks behind because we do not know the french Canadian idioms

expansion_rules:
appele: "(appelé|nommé|surnommé|pour [<le>])"
appele: "(appelé[e]|[sur]nommé[e]|pour [<le>])"
Copy link
Contributor

@jlpouffier jlpouffier Apr 30, 2025

Choose a reason for hiding this comment

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

Let's avoid complexifying expansion rules. Even if what you did is more compact, it's more complex to read for contributors.
Golden rule for us is to keep the intent repo as legible as possible.

expansion_rules:
appele: "(appelé|nommé|surnommé|pour [<le>])"
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment as above, let's revert that particular change for legibility

expansion_rules:
appele: "(appelé|nommé|surnommé|pour [<le>])"
appele: "(appelé[e]|[sur]nommé[e]|pour [<le>])"
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment as above, let's revert that particular change for legibility

expansion_rules:
appele: "(appelé|nommé|surnommé|pour [<le>])"
appele: "(appelé[e]|[sur]nommé[e]|pour [<le>])"
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment as above, let's revert that particular change for legibility

@@ -54,7 +54,7 @@ intents:
# Minuteur d'une heure appelé Pizza
- "<minuteur> d'<timer_duration> <appele> {timer_name:name}"
expansion_rules:
appele: "(appelé|nommé|surnommé|pour [<le>])"
appele: "(appelé[e]|[sur]nommé[e]|pour [<le>])"
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment as above, let's revert that particular change for legibility

@@ -101,7 +101,7 @@ intents:
# Mets un minuteur pour fermer la fenêtre dans 5 minutes
- "<mets> un <minuteur> pour {timer_name:name} dans <timer_duration>"
expansion_rules:
appele: "(appelé|nommé|surnommé|pour [<le>])"
appele: "(appelé[e]|[sur]nommé[e]|pour [<le>])"
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment as above, let's revert that particular change for legibility

@@ -66,4 +66,4 @@ intents:
- "[Il reste] [encore] combien de temps avant (de |d'|<le>){timer_name:name}"
expansion_rules:
reste_t_il: "(reste-t-il)|(reste t il)"
appele: "(appelé|nommé|surnommé|pour [<le>])"
appele: "(appelé[e]|[sur]nommé[e]|pour [<le>])"
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment as above, let's revert that particular change for legibility

expansion_rules:
appele: "(appelé|nommé|surnommé|pour [<le>])"
appele: "(appelé[e]|[sur]nommé[e]|pour [<le>])"
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment as above, let's revert that particular change for legibility

@home-assistant home-assistant bot marked this pull request as draft April 30, 2025 13:13
@jbblanchet jbblanchet requested a review from jlpouffier April 30, 2025 17:59
@jbblanchet jbblanchet marked this pull request as ready for review April 30, 2025 17:59
@jbblanchet
Copy link
Contributor Author

Additional question: Are you located in Canada? We're looking for contributors in Canada because we're afraid to leave you folks behind because we do not know the french Canadian idioms

I am indeed Québécois. I'd be happy to help with French Canadian specific expressions. I don't have a lot of spare time, but there doesn't seem to be that much intents being added. I'll try to find some time next week to review existing files to see if anything obvious.

Thanks for your hard work, I`m very happy with my 2 new Voice Previews (even if they were tricky to get all the way up here 😄)

@jlpouffier
Copy link
Contributor

Additional question: Are you located in Canada? We're looking for contributors in Canada because we're afraid to leave you folks behind because we do not know the french Canadian idioms

I am indeed Québécois. I'd be happy to help with French Canadian specific expressions. I don't have a lot of spare time, but there doesn't seem to be that much intents being added. I'll try to find some time next week to review existing files to see if anything obvious.

Thanks for your hard work, I`m very happy with my 2 new Voice Previews (even if they were tricky to get all the way up here 😄)

Great !
PR approved. The intents have just been cut because today is beta week but I'll make sure this is included in the stable release next week

Good job,
I'll look in priority all PRs coming from you if they make the life of Canadians better. This has always been top of my mind.

Take care !

@jlpouffier jlpouffier enabled auto-merge (squash) April 30, 2025 18:11
@jlpouffier jlpouffier merged commit dae745c into home-assistant:main Apr 30, 2025
2 checks passed
@andreasbrett andreasbrett changed the title Add "minuterie" french translation for timer [FR] Add "minuterie" french translation for timer May 1, 2025
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.

2 participants