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

Split out gpslogger into a separate component and platform #20044

Merged
merged 4 commits into from Jan 14, 2019

Conversation

Projects
None yet
6 participants
@rohankapoorcom
Copy link
Member

rohankapoorcom commented Jan 13, 2019

Description:

Splits the gpslogger device tracker to be a component in addition to the device_tracker platform. This is required to migrate it over to use the webhook component.

Breaking Change: The gpslogger device_tracker platform no longer takes any configuration. The configuration needs to be applied to the component gpslogger instead. The platform will be automatically loaded with the component and should not be specified in configuration.yaml.

Related issue (if applicable): fixes #

Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#8152

Example entry for configuration.yaml (if applicable):

gpslogger:

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • New dependencies have been added to the REQUIREMENTS variable (example).
  • New dependencies are only imported inside functions that use them (example).
  • New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • New files were added to .coveragerc.

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.
@MartinHjelmare
Copy link
Member

MartinHjelmare left a comment

Looks good!

@MartinHjelmare
Copy link
Member

MartinHjelmare left a comment

We should update coveragerc too. What's the coverage? Can we remove the device tracker platform or should we add the component?

@rohankapoorcom

This comment has been minimized.

Copy link
Member Author

rohankapoorcom commented Jan 13, 2019

The device tracker platform is currently in .coveragerc. The component is at 83.33%

I can add a couple more component tests and I think we can remove the device tracker platform from .coveragerc.

@MartinHjelmare
Copy link
Member

MartinHjelmare left a comment

Nice!

@MartinHjelmare

This comment has been minimized.

Copy link
Member

MartinHjelmare commented Jan 13, 2019

Can be merged when build passes.

@rohankapoorcom

This comment has been minimized.

Copy link
Member Author

rohankapoorcom commented Jan 13, 2019

Build seems to have failed due to unrelated things, will close/reopen to trigger another.

@wafflebot wafflebot bot removed the in progress label Jan 13, 2019

@wafflebot wafflebot bot added the in progress label Jan 13, 2019

@MartinHjelmare

This comment has been minimized.

Copy link
Member

MartinHjelmare commented Jan 13, 2019

If it's a known flaky test we can just ignore it. But let's see what this build has to offer.

@rohankapoorcom

This comment has been minimized.

Copy link
Member Author

rohankapoorcom commented Jan 13, 2019

Looks like the build succeeded this time. We have 100% coverage on both the component and the platform.

@MartinHjelmare MartinHjelmare merged commit 7f38710 into home-assistant:dev Jan 14, 2019

5 checks passed

Hound No violations found. Woof!
WIP Legacy commit status override — see details
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.002%) to 93.031%
Details

@wafflebot wafflebot bot removed the in progress label Jan 14, 2019

@rohankapoorcom rohankapoorcom deleted the rohankapoorcom:gpslogger-component branch Jan 14, 2019

@balloob balloob removed the new-platform label Jan 21, 2019

@balloob balloob referenced this pull request Jan 23, 2019

Merged

0.86.0 #20354

alandtse added a commit to alandtse/home-assistant that referenced this pull request Feb 12, 2019

Split out gpslogger into a separate component and platform (home-assi…
…stant#20044)

* Split out gpslogger into a separate component and platform

* Lint

* Lint

* Increase test coverage
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment