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

Fix model dir #10164

Merged
merged 5 commits into from
Nov 10, 2021
Merged

Fix model dir #10164

merged 5 commits into from
Nov 10, 2021

Conversation

tayfun
Copy link
Contributor

@tayfun tayfun commented Nov 10, 2021 •
edited
Loading

Fixes #10101

Status (please check what you already did):

  • added some tests for the functionality
  • updated the documentation
  • updated the changelog (please check changelog for instructions)
  • reformat files using black (please check Readme for instructions)

@tayfun tayfun requested a review from a team as a code owner November 10, 2021 15:13
@tayfun tayfun requested review from carlad and removed request for a team November 10, 2021 15:13
@tayfun tayfun requested a review from joejuzl November 10, 2021 16:21
@carlad
Copy link
Contributor

carlad commented Nov 10, 2021

LGTM 👍
Re: the check boxes in the description. Are we supposed to check them even if we haven't updated documentation, updated the changelog, or added tests?

@tayfun
Copy link
Contributor Author

tayfun commented Nov 10, 2021

LGTM 👍 Re: the check boxes in the description. Are we supposed to check them even if we haven't updated documentation, updated the changelog, or added tests?

Haha I think so :D

I mean I check boxes for items done or not applicable. It's just a system to check you have considered all in my opinion.

Docs need not changed here as it is a bugfix, changelog is not needed because it is not a bug that's been released (only in RC yet), and I think again tests are not needed as I'm only removing buggy code.

@joejuzl
Copy link
Contributor

joejuzl commented Nov 10, 2021 •
edited
Loading

Are we supposed to check them even if we haven't updated documentation, updated the changelog, or added tests?

I normally remove the whole line if it's not applicable.

joejuzl
joejuzl approved these changes Nov 10, 2021
View reviewed changes
Copy link
Contributor

@joejuzl joejuzl left a comment

Choose a reason for hiding this comment

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

💯

@tayfun tayfun enabled auto-merge (squash) November 10, 2021 17:12
@tayfun tayfun merged commit 8ac3c54 into main Nov 10, 2021
@tayfun tayfun deleted the tayfun/10101-fix-model-dir branch November 10, 2021 18:03
aeshky added a commit that referenced this pull request Nov 11, 2021
@aeshky
Merge branch 'markers_bugfix' of https://github.com/RasaHQ/rasa into …
6d67686 
…markers_bugfix

* 'markers_bugfix' of https://github.com/RasaHQ/rasa:
  Fix model dir (#10164)
  add changelog entry about tf2.6 update (#10153)
  conversation_granularity for slack - per user | per user per channel | per user per thread (#10086)
  add changelog fragment for docs changes in 3.0
  fix syntax in training data example in the docs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@carlad carlad carlad approved these changes

@joejuzl joejuzl joejuzl approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Model file not recognized in rasa shell for 3.0 RC1
3 participants
@tayfun @carlad @joejuzl