Skip to content

Add Buienradar camera for Belgium#30399

Merged
springstan merged 7 commits into
home-assistant:devfrom
lade-odoo:dev
Jan 24, 2020
Merged

Add Buienradar camera for Belgium#30399
springstan merged 7 commits into
home-assistant:devfrom
lade-odoo:dev

Conversation

@lade-odoo
Copy link
Copy Markdown
Contributor

Description:

Pull request with documentation for home-assistant.io (if applicable): home-assistant/home-assistant.io/pull/11604

Example entry for configuration.yaml (if applicable):

camera:
  - platform: buienradar
    country_code: BE

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.
  • I have followed the development checklist

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

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

@homeassistant
Copy link
Copy Markdown
Contributor

Hi @Minilau,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

@probot-home-assistant
Copy link
Copy Markdown

Hey there @mjj4791, @ties, mind taking a look at this pull request as its been labeled with a integration (buienradar) you are listed as a codeowner for? Thanks!

@lade-odoo lade-odoo changed the title Vote Buienradar camera for Belgium Buienradar camera for Belgium Jan 2, 2020
Copy link
Copy Markdown
Contributor

@ties ties left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good.

There is no test for this functionality and I can not add commits to the pull request. You could add one (and clean up the split string added by black in test_camera.py) if you want.

Comment thread homeassistant/components/buienradar/camera.py Outdated
@springstan springstan changed the title Buienradar camera for Belgium Add Buienradar camera for Belgium Jan 2, 2020
@ties
Copy link
Copy Markdown
Contributor

ties commented Jan 3, 2020

👍 nice additional feature. Was not aware of the Belgian maps when I added the camera.

@lade-odoo
Copy link
Copy Markdown
Contributor Author

Shouldn't the PR be marked as reviewed now that everything is okay ?

@ties
Copy link
Copy Markdown
Contributor

ties commented Jan 7, 2020

Shouldn't the PR be marked as reviewed now that everything is okay ?

Yes. But I can not do that :S. @frenck (due to timezone)?

Comment thread homeassistant/components/buienradar/camera.py Outdated
Comment thread homeassistant/components/buienradar/camera.py Outdated
Comment thread tests/components/buienradar/test_camera.py Outdated
Copy link
Copy Markdown
Member

@frenck frenck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a couple of small comments.

@lade-odoo
Copy link
Copy Markdown
Contributor Author

I have changed what you requested ! Thank you for the review @frenck

@springstan springstan merged commit 6ff572d into home-assistant:dev Jan 24, 2020
@lock lock Bot locked and limited conversation to collaborators Jan 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants