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

Reddit Sensor #21344

Merged
merged 3 commits into from Feb 26, 2019

Conversation

Projects
None yet
6 participants
@ljmerza
Copy link
Contributor

ljmerza commented Feb 23, 2019

Description:

Add Reddit Sensor

Related issue (if applicable): fixes #

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

Example entry for configuration.yaml (if applicable):

Checklist:

  • [ x] The code change is tested and works locally.
  • [ x] Local tests pass with tox. Your PR cannot be merged unless tests pass
  • [ x] 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:

  • [ x] New dependencies have been added to the REQUIREMENTS variable (example).
  • [ x] New dependencies are only imported inside functions that use them (example).
  • [ x] New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • [ x] 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.
@fabaff

This comment has been minimized.

Copy link
Member

fabaff commented Feb 23, 2019

Success of #21297

Show resolved Hide resolved homeassistant/components/sensor/reddit.py Outdated
Show resolved Hide resolved homeassistant/components/sensor/reddit.py Outdated
Show resolved Hide resolved homeassistant/components/sensor/reddit.py Outdated
Show resolved Hide resolved homeassistant/components/sensor/reddit.py Outdated
Show resolved Hide resolved homeassistant/components/sensor/reddit.py Outdated

@ljmerza ljmerza requested a review from home-assistant/core as a code owner Feb 24, 2019

Show resolved Hide resolved homeassistant/components/sensor/reddit.py Outdated
Show resolved Hide resolved homeassistant/components/sensor/reddit.py Outdated
Show resolved Hide resolved homeassistant/components/sensor/reddit.py Outdated
Show resolved Hide resolved homeassistant/components/sensor/reddit.py Outdated
Show resolved Hide resolved homeassistant/components/sensor/reddit.py Outdated
Show resolved Hide resolved homeassistant/components/sensor/reddit.py Outdated
Show resolved Hide resolved homeassistant/components/sensor/reddit.py Outdated
Show resolved Hide resolved homeassistant/components/sensor/reddit.py Outdated
Show resolved Hide resolved homeassistant/components/sensor/reddit.py Outdated
Show resolved Hide resolved homeassistant/components/sensor/reddit.py Outdated
@MartinHjelmare

This comment has been minimized.

Copy link
Member

MartinHjelmare commented Feb 25, 2019

We should move the platform module under a reddit component package, since this is a new platform.

homeassistant/components/reddit/__init__.py
homeassistant/components/reddit/sensor.py

The init module should just have docstring. The sensor module should be the current reddit sensor platform.

@ljmerza ljmerza referenced this pull request Feb 25, 2019

Merged

Reddit Sensor #8675

@ljmerza ljmerza force-pushed the ljmerza:reddit branch from d79d86d to 4b9f1be Feb 25, 2019

@ljmerza

This comment has been minimized.

Copy link
Contributor Author

ljmerza commented Feb 25, 2019

We should move the platform module under a reddit component package, since this is a new platform.

homeassistant/components/reddit/__init__.py
homeassistant/components/reddit/sensor.py

The init module should just have docstring. The sensor module should be the current reddit sensor platform.

Moved the code over.

@MartinHjelmare
Copy link
Member

MartinHjelmare left a comment

Looks good!

@fabaff?

@MartinHjelmare

This comment has been minimized.

Copy link
Member

MartinHjelmare commented Feb 25, 2019

requirements_all.txt is not up to date. Please run script/gen_requirements_all.py.

@ljmerza

This comment has been minimized.

Copy link
Contributor Author

ljmerza commented Feb 25, 2019

requirements_all.txt is not up to date. Please run script/gen_requirements_all.py.

When I run script/gen_requirements_all.py It makes a ton of changes. Do you know why this is? I ran it before and all it did was update the praw

@MartinHjelmare

This comment has been minimized.

Copy link
Member

MartinHjelmare commented Feb 25, 2019

You probably need to rebase the branch on latest dev branch.

@ljmerza ljmerza force-pushed the ljmerza:reddit branch from cdbb429 to 49dc7a1 Feb 25, 2019

@ljmerza

This comment has been minimized.

Copy link
Contributor Author

ljmerza commented Feb 25, 2019

You probably need to rebase the branch on latest dev branch.

ok thank you. Updated requirements_all.txt

@ljmerza ljmerza force-pushed the ljmerza:reddit branch from f6159e3 to b9481a8 Feb 25, 2019

ljmerza added some commits Feb 23, 2019

init
automated commit 22/02/2019 22:55:49

cr comments

cr comments

automated commit 24/02/2019 14:41:08

automated commit 24/02/2019 14:41:59

automated commit 24/02/2019 14:54:16

automated commit 24/02/2019 14:54:49

automated commit 24/02/2019 19:46:15

automated commit 25/02/2019 10:10:46

automated commit 25/02/2019 10:10:52

automated commit 25/02/2019 10:12:16

automated commit 25/02/2019 10:15:59

@ljmerza ljmerza force-pushed the ljmerza:reddit branch from 8fb7c6c to bb16092 Feb 26, 2019

@balloob balloob merged commit e739fd8 into home-assistant:dev Feb 26, 2019

3 of 4 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
Hound No violations found. Woof!
cla-bot Everyone involved has signed the CLA
coverage/coveralls Coverage remained the same at 92.741%
Details

@wafflebot wafflebot bot removed the in progress label Feb 26, 2019

@ljmerza ljmerza deleted the ljmerza:reddit branch Feb 26, 2019

@balloob balloob referenced this pull request Mar 6, 2019

Merged

0.89.0 #21712

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.