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

Jump to bottom

Add documentation for new RMV public transport sensor #5947

Merged
merged 12 commits into from Aug 13, 2018
Merged

Add documentation for new RMV public transport sensor #5947

merged 12 commits into from Aug 13, 2018

Conversation

cgtobi
Copy link
Contributor

@cgtobi cgtobi commented Aug 3, 2018 •
edited

Description:
This adds documentation for a new public transport sensor for the Rhein-Main area. It is based upon PyRMVtransport which uses public data from opendata.rmv.de.

Pull request in home-assistant (if applicable): home-assistant/core#15814

Checklist:

  • Branch: Fixes, changes and adjustments should be created against current. New documentation for platforms/components and features should go to next.
  • The documentation follow the standards.

@ghost ghost added the to-do label Aug 3, 2018
@cgtobi cgtobi mentioned this pull request Aug 3, 2018
Merged
8 tasks
@frenck frenck added new-integration This PR adds documentation for a new Home Assistant integration ready-for-review This PR needs to be reviewed next This PR goes into the next branch has-parent This PR has a parent PR in a other repo and removed to-do labels Aug 4, 2018
@MartinHjelmare MartinHjelmare changed the title Add documentation for new RMV public transport sensor. Add documentation for new RMV public transport sensor Aug 10, 2018
MartinHjelmare
MartinHjelmare reviewed Aug 10, 2018
View reviewed changes
source/_components/sensor.rmvtransport.markdown Outdated
- station: STATION_OR_STOP_ID
```

Configuration variables:
Copy link
Member

Choose a reason for hiding this comment

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

Please use our configuration tag syntax instead to describe the config options.

https://developers.home-assistant.io/docs/en/documentation_create_page.html#configuration

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. Thanks for the info. I updated the config options.

@MartinHjelmare
Copy link
Member

Looks like the build failed on an unrelated page.

DavidMStraub
DavidMStraub reviewed Aug 11, 2018
View reviewed changes
source/_components/sensor.rmvtransport.markdown Outdated
# Example configuration.yaml entry
sensor:
- platform: rmvtransport
nextdeparture:
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this called next_departure in the merged version? (Which by the way is not consistent with the MVG sensor, where we have nextdeparture and timeoffset. Not sure if this should be handled consistently.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank's for pointing it out. I agree with your remark about consistency, but it actually is PEP 8 compliant. So it would rather make sense to change the names of the MVG sensor, no?

@frenck frenck self-assigned this Aug 13, 2018
@frenck frenck added this to the 0.76 milestone Aug 13, 2018
@frenck
Copy link
Member

frenck commented Aug 13, 2018

This needs to be rebased in order to fix the build. Will take care of that.

@frenck
Copy link
Member

frenck commented Aug 13, 2018

Rebased, awaiting build results before starting a review.

@frenck frenck added awaits-parent Awaits the merge of an parent PR and removed ready-for-review This PR needs to be reviewed labels Aug 13, 2018
@frenck
Copy link
Member

frenck commented Aug 13, 2018

Looks good to go! 🎉

@frenck frenck merged commit ee1c05e into home-assistant:next Aug 13, 2018
@ghost ghost removed the awaits-parent Awaits the merge of an parent PR label Aug 13, 2018
@frenck frenck removed their assignment Aug 13, 2018
@balloob balloob added the cherry-picked This PR has been manually picked and merged into the current branch label Aug 14, 2018
balloob pushed a commit that referenced this pull request Aug 14, 2018
@cgtobi @balloob
Add documentation for new RMV public transport sensor (#5947)
c315ed2 
* Initial documentation for the rmvtransport sensor.

* Fix name

* Fix typo

* Added missing variable description for max number of journeys.

* Minor wording fix

* Fix setting names after code change.

* Use configuration tag syntax.

* Add logo.

* Update logo file name.

* Fix liquid syntax error.

* Fix config setting name.

* ✏️ Markdown tweaks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MartinHjelmare MartinHjelmare MartinHjelmare left review comments

@DavidMStraub DavidMStraub DavidMStraub left review comments

@frenck frenck frenck approved these changes

Assignees
No one assigned
Labels
cherry-picked This PR has been manually picked and merged into the current branch has-parent This PR has a parent PR in a other repo new-integration This PR adds documentation for a new Home Assistant integration next This PR goes into the next branch
Projects
None yet
Milestone
0.76
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@cgtobi @MartinHjelmare @frenck @DavidMStraub @balloob