-
Notifications
You must be signed in to change notification settings - Fork 71.9k
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
Add notes about the Google Assistant integration being broken #8076
Conversation
Update 10/16/19
Update README.md
14.2.1 update
Thanks for confirming, should we remove the code and log a deprecation warning as well? |
We could, but I'm wondering if it might be helpful to keep it in case we can revive it again, or if someone figures out a good way to use one of Google's alternatives to Conversational Actions. As far as I'm aware, it doesn't expose any security vulnerabilities, and it doesn't add overhead to the application. The code that supports Google Assistant specifically is just a "translator" that sits between the Google APIs and the generic virtual assistant code in Nightscout. The Alexa integration also depends on the generic part of the code, so we at least don't want to remove that part for sure. |
I'm interested in otherwise comparable alternatives. To the extent we can remove any dependencies, we would like to do so, especially non-functional ones. We've spent the better part of the last year working on bringing dependencies up to date and to prepare for removing things that aren't going to help us move forward. It makes sense to me to remove all the google-assistant specific code and replace it with a warning on the console, especially if it allows us to remove a dependency, especially any additional http libraries. |
Good to know. Thankfully there's no dependencies |
Whoops, wrong button. There's no dependencies that are unique to the Google Assistant integration, so we're clear there. I suppose we could remove the code, and maybe I'll stick a link in the docs to the last commit with the code. That way anyone who figures out a good way to do it again can use it as a reference. |
In what context do you think a warning log message will be needed? Like if they have the plugin in the enabled list? |
Yes, if someone enables the plugin, what should happen? |
As it stands right now, if the plugin is enabled, the app will have the routes setup for this plugin. Since nothing will be making calls to those API endpoints, nothing will be happening after that. There's no "active" part to this plugin; it's all "reactive" since nothing happens until a request is made to that specific endpoint. |
No description provided.