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.

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

moodbot with rules & yml files #6229

Closed
wants to merge 7 commits into from
Closed

moodbot with rules & yml files #6229

wants to merge 7 commits into from

Conversation

ArjaanBuijk
Copy link
Contributor

Proposed changes:

  • Use rules for greet, goodbye & bot_challenge
  • Leave the happy & sad paths as stories

@ArjaanBuijk ArjaanBuijk requested a review from akelad July 16, 2020 20:50
@akelad
Copy link
Contributor

akelad commented Jul 17, 2020

@ArjaanBuijk thanks for submitting that :D The stories also need to be converted to the yaml format, and so does the nlu training data - could you do that as well? not sure if any other changes are needed e.g. in the domain file, but it's good to take a look at the rules folder in the examples

@ArjaanBuijk ArjaanBuijk changed the title Use rules for greet, goodbye & challenge moodbot with rules & yml files Jul 17, 2020

- story: sad path 1
steps:
- intent: mood_unhappy
Copy link
Contributor

Choose a reason for hiding this comment

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

i actually wonder if this is the correct split here 🤔 because "mood_unhappy" is the direct response to the bots greeting (it says "hi how are you" or something)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@akelad ,

I restored the greet->utter_greet as starting point of all the stories, and removed the corresponding rule.

Copy link
Contributor

@akelad akelad left a comment

Choose a reason for hiding this comment

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

looks good to me now! It seems like a bunch of tests are failing though (I guess they rely on moodbot). Do you have time to take a look at fixing them?

@akelad
Copy link
Contributor

akelad commented Jul 29, 2020

@ArjaanBuijk do you have time to fix the tests?

@tmbo
Copy link
Member

tmbo commented Aug 3, 2020

I based my PR in #6323 on this and I think I fixed the tests (we'll see if CI passes, fingers crossed). so might make sense to close this one and just merge the other one.

@akelad
Copy link
Contributor

akelad commented Aug 3, 2020

sounds fine to me

@akelad akelad closed this Aug 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants