[EN] Add sentence option "an" #2847
Conversation
dlearningcode
requested review from
mrdarrengriffin,
tetele and
ViViDboarder
as code owners
![home-assistant[bot]](https://avatars.githubusercontent.com/in/97978?s=60&v=4){kind=small}
There was a problem hiding this comment.
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!
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
home-assistant
bot
added
cla-recheck
cla-signed
and removed
cla-recheck
cla-needed
labels
|
CLA signed, pull request review process page read. Sorry, learning where everything is as written instructions perforce are always out of date. |
|
Please forgive any annoyance here. This is my very first time ever forking a repository and creating a pull request, so I've been bumbling about among various resources to try to get this done. Right now the thread is reporting that 1 workflow is awaiting approval; a build whose status needs to be reported. What does that mean and how do I do it? This isn't mentioned in any of the links I've clicked through on this page, and ChatGPT says to ask a maintainer. |
andreasbrett
changed the title
|
Looks good but you should also add tests for these new/modified sentences. The reviewers for EN should soon review and approve this. You're almost there. |
Hey Andreas, thanks for the input. I would happily add tests, but: 1) All I did was add an option, without changing the structure, so I didn't think it would require any additional consideration; 2) I don't know how to do tests. I had learned a little about them when I first got my front end developer certificate, but I've never actually written or applied them in any environment. I wouldn't even know where to begin. |
|
Take a look at this and make a copy of that block using 8 instead Here's how to run the tests: https://github.com/home-assistant/intents/tree/main?tab=readme-ov-file#run-tests |
This is fantastic information in quick review, ViViDboarder! I'm not exactly sure what to do with it; I have no familiarity with Python. I'm trying to research (I mostly learn through YouTube), but if you don't mind and have a moment, am I supposed to add new sentences to the StartTimer page and then test them in VS Code? Scanning through the StartTimer page, I can see that there were no examples with numbers starting with vowels. I will continue to learn, but will this hold up the approval process? |
|
You don't actually need to write any Python. It's all just sentences in these yaml files. A Python script is going to run the tests, but you only need to execute it. If you're using vscode, you should be able to use the devcontainer (I think vscode asks you about this automatically? I don't use vscode or devcontainers myself). Once you edit the yaml file you can enter a shell in the dev container and run the tests by running |
|
Nice suggestion. Looks great |
|
@dlearningcode The only suggestion I would make is to add some acompanying tests for the new addition |
|
Thanks for approving, @mrdarrengriffin. @ViViDboarder and andreasbrett made the same suggestion about tests, and I want to comply, but this is the first I've heard of this and I'm trying to wrap my head around how to do it. I'm now starting to go through the devcontainer documentation, and will hope to get that done, whatever that entails, today or tomorrow. |
|
Awesome. Thanks for your patience. If you need help, you can check out the Home Assistant Discord in the developer support forum. |
|
Easiest way to use devcontainers would be to start a Github codespace for your fork of the intents repo
|
Sorry, been down sick, just started researching again a bit last night. @tetele, I very much appreciate the tip, but I will admit it confuses me, as the directions on using codespaces say to run the codespace directly from I was actually trying to learn how to build a devcontainer in my local VS Code, so that I can work this from my different computers at home -- I've actually already forked the After all that, there's still writing the test! I'm reading the developer docs on template sentence syntax, test syntax, and contributing sentences, and will bumble my way through, as usual. Now, if you direct me to the discord, I'm happy to do that, but if you're willing to address any/all of my questions, I'd be grateful. |
I am suggesting building it from a repo where you have write access, i.e. your own fork
In your codespace or in VSCode, open up a terminal and write the command while you're in your workspace folder
#developers on Discord. You may get redirected from there |
|
Thanks, @tetele, I have started a request at Discord. If I create a codespace from my fork, will I be able to follow the directions on codespaces built from home-assistant/intents to then create the pull request, or does anything change if I'm doing it on my fork? |
|
The instructions are pretty much solid. Maybe some small things are a bit outdated, but all in all you should be good to go. |
|
Hey Team, I'm looking for guidance committing my codespace sentence tests update. I opened a codespace on this According to the codespace instructions, I should select Commit & Create Pull Request, but that doesn't seem to apply here as this is already a PR. Guidance on proper selection, please?
|
|
Okay, I think I did everything right, I believe I'm up to date with the main branch, and the tests have been added. But the status thing shows "1 workflow awaiting approval" and "Required statuses must pass before merging". I clicked the statuses link but didn't understand any of what was there. Could you please let me know if there's something else I need to do? |
I decided to do Commit & Push. Let me know if that was wrong. |
Add "an" as an option to sentences, for calling numbers that begin with vowels, such as eight, eleven, or eighty. Currently, Assist fails if I ask it to "set an eleven minute timer".