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

Handle removing unregistered intents #2235

Merged
merged 1 commit into from Jul 31, 2019

Conversation

@forslund
Copy link
Member

commented Jul 26, 2019

Description

This small change adds a check that the intent has been registered
before removing it.

If an unregistered intent was removed padatious would silently throw an
exception due to a list operation error. But when run synchronously from
the skill tester this silent exception was actually loud causing skills
to fail to load.

How to test

Run the skill tester on the latest beta of the npr-news skill.

Contributor license agreement signed?

CLA [ Yes ]

This small change adds a check that the intent has been registered
before removing it.

If an unregistered intent was removed padatious would silently throw an
exception due to a list operation error. But when run synchronously from
the skill tester this silent exception was actually loud causing skills
to fail to load.
@krisgesling

This comment has been minimized.

Copy link
Contributor

commented Jul 31, 2019

Looks good to me. Merging.

@krisgesling krisgesling merged commit b357874 into dev Jul 31, 2019
3 checks passed
3 checks passed
:-) Build finished.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.003%) to 51.179%
Details
@forslund forslund deleted the bugfix/padatious-detach-unregistered branch Aug 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.