-
-
Notifications
You must be signed in to change notification settings - Fork 29.1k
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
Migrate geo_json_events platform to integration with config flow #70439
Migrate geo_json_events platform to integration with config flow #70439
Conversation
194bf5f
to
2f5b17a
Compare
31dea63
to
668c359
Compare
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. |
PR is still current, ready to be reviewed and merged. |
51d6adf
to
2fe8b9f
Compare
Just rebased to latest dev branch. |
dd3db46
to
c5745db
Compare
Rebased again. I ran tox locally with py39 and p310 and all tests ran fine, i.e. I couldn't reproduce the previously failed tests. |
c5745db
to
a5a45f8
Compare
Rebased again; added integration type; replaced deprecated unit system. |
hass.async_create_task( | ||
hass.config_entries.flow.async_init( | ||
DOMAIN, context={"source": SOURCE_IMPORT}, data=config | ||
) |
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.
Very quick scan, but a couple of things:
- I think it should raise an issue to tell the user to remove the manual configuration
- config_flow PRs are always quite big. I suggest that you remove everything else can can be kept for a follow-up PR (eg. the
sensor
platform)
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.
Thanks. I'll have a look into raising an issue from this step. And, let me have a look how I can simplify this PR to only include configflow for now, and move the other small improvements to a future PR.
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.
I followed your advice and reduced this change to the minimum, i.e. config flow only, and this is now available here: #87062
Closing in favour of #87062 |
Closing in favour of #87062 |
Breaking change
Proposed change
Migrate geo_json_events from a platform to an integration with config flow. This integration uses the data coordinator to manage entities. Also introduces a diagnostics sensor that keeps track of updates from the external feed.
The legacy way of defining this integrations as a geo_location platform via YAML configuration is still supported and the configuration will automatically be migrated.
Type of change
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: