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.

Sign up for GitHub

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

Jump to bottom

Add zodiac integration #38935

Merged
merged 28 commits into from
Sep 21, 2020
Merged

Add zodiac integration #38935

merged 28 commits into from
Sep 21, 2020

Conversation

JulienTant
Copy link
Contributor

@JulienTant JulienTant commented Aug 16, 2020 •
edited
Loading

Proposed change

This new component add a new sensor representing the current zodiac sign.

Also provide some attributes as:

  • the sign itself
  • the associated element
  • the associated modality

The informations come from Wikipedia

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example entry for configuration.yaml:

# Example configuration.yaml
sensor:
  - platform: zodiac

Additional information

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

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

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

To help with the load of incoming pull requests:

Note from the author

This is my first pull request and I'm not used to develop in python, I'll accept any kind of comment to help improve the quality of my code. Thank you for your time.

@homeassistant
Copy link
Contributor

Hi @JulienTant,

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!

@JulienTant JulienTant mentioned this pull request Aug 16, 2020
Merged
8 tasks
@JulienTant JulienTant marked this pull request as ready for review August 16, 2020 14:18
ctalkington
ctalkington reviewed Sep 7, 2020
View reviewed changes
homeassistant/components/zodiac/translations/sensor.en.json Outdated
@@ -0,0 +1,18 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

these translations should be done via lokalise rather than directly (will be overrriden nightly)

ctalkington
ctalkington requested changes Sep 20, 2020
View reviewed changes
Copy link
Contributor

@ctalkington ctalkington left a comment

Choose a reason for hiding this comment

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

holding on merge due to endless loop in tests

MartinHjelmare
MartinHjelmare requested changes Sep 21, 2020
View reviewed changes
homeassistant/components/zodiac/sensor.py Outdated Show resolved Hide resolved
homeassistant/components/zodiac/sensor.py Outdated Show resolved Hide resolved
homeassistant/components/zodiac/translations/sensor.fr.json Outdated Show resolved Hide resolved
@MartinHjelmare MartinHjelmare self-assigned this Sep 21, 2020
@MartinHjelmare MartinHjelmare removed their assignment Sep 21, 2020
MartinHjelmare
MartinHjelmare approved these changes Sep 21, 2020
View reviewed changes
Copy link
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

Looks good!

@ctalkington ctalkington merged commit 8b9c757 into home-assistant:dev Sep 21, 2020
@ctalkington ctalkington changed the title Add zodiac sign sensor Add zodiac integration Sep 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MartinHjelmare MartinHjelmare MartinHjelmare approved these changes

@ctalkington ctalkington ctalkington approved these changes

Assignees
No one assigned
Labels
cla-signed has-tests integration: zodiac new-integration
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@JulienTant @homeassistant @ctalkington @MartinHjelmare