-
-
Notifications
You must be signed in to change notification settings - Fork 34.3k
Add config flow for Waze Travel Time #43419
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
Conversation
My $0.02: I wouldn't add a config flow and remove YAML configuration in the same PR. I would split that work between 3 PRs:
|
I am happy to do that but it won't prevent a breaking change. In order to support config flow, this has to move to a top level config item. |
You don't need to change the schema to support config entries. See how Tile does it (using the |
ah this is clever, thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please seem comments above.
Thanks 👍
this will need a breaking change notice, even though it technically isn't breaking, as its used when creating release blog post. |
Why is that? Doesn't seem like a pattern that is typically used. As an example, for DirectTV the breaking change notice was only posted after import configuration was removed |
Our standard is to mark both the deprecation and the invalidation PR as breaking. |
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. |
00970a1
to
f39de59
Compare
still valid |
Co-authored-by: Martin Hjelmare <marhje52@gmail.com>
* add config flow support to google_travel_time * fix bugs and add strings * fix import and add new test * address comments in #43419 since this is a similar PR * fix default name and test * add unique ID and device info * fix test * feedback from waze PR * continue incorporating feedback from waze PR * final fixes and update tests * call update in lambda * Update homeassistant/components/google_travel_time/sensor.py Co-authored-by: Martin Hjelmare <marhje52@gmail.com> * additional fixes * validate config entry data during config flow and config entry setup * don't store entity * patch dependency instead of HA code * fixes * improve tests by moving all patching to fixtures * use self.hass instead of setting self._hass * invert if * remove unnecessary else Co-authored-by: Martin Hjelmare <marhje52@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Breaking Change
Config flow support has been added to
waze_travel_time
. Existing YAML configuration entries will be imported automatically but should be removed from YAML configuration after import. Import support will be removed in a future release.Proposed change
Adds config flow support to
waze_travel_time
. The original platform setup strategy has been left around via config flow import to make the transition easier per @bachya 's recommendation below. The plan is to deprecate it then remove it in a future release.Type of change
Example entry for
configuration.yaml
:N/A
Additional information
Checklist
black --fast homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: