Skip to content
This repository was archived by the owner on Jul 9, 2025. It is now read-only.

Conversation

@lei9444
Copy link
Contributor

@lei9444 lei9444 commented Nov 8, 2019

Description

Check error.

Task Item

Closes #1535

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code refactor (non-breaking change which improve code quality, clean up, add tests, etc)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Doc update (document update)

Checklist

  • I have added tests that prove my fix is effective or that my feature works
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have functionally tested my change

Screenshots

Please include screenshots or gifs if your PR include UX changes.

@boydc2014 boydc2014 changed the title Fix: Failure to create a bot from scratch Fix start bot failure after switching reconginzer from Luis to None Nov 8, 2019
@boydc2014
Copy link
Contributor

This did fixed the issue, but i wonder why we deal with luis config here, since we are not using luisRecongizer.

I'm not suggesting any immediate change here. It's totally OK that we want pass a placeholder as luisconfig to fit into the interface of 'start bot'.

Just want to make sure we leverage each issue & fix as a chance to review the code and the design. Like the similar question i posted here: #1215 (comment) . I hope you and @liweitian can spend some times to review the logic you guys implemented. See what we can improve.

Common practices we can use is

  • Adding comments to the logic, when you add comments you will notice 'hard to describe what happend here", it's a signal of bad design
  • Adding tests. When you find that, "testing one component requires 10 depenecenies" , it's also a signal.

@boydc2014 boydc2014 merged commit 81bfbc4 into microsoft:master Nov 8, 2019
@lei9444 lei9444 deleted the luis branch January 17, 2020 07:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants