Skip to content

Update Swiss Hydrological Data documentation#6535

Merged
fabaff merged 4 commits intohome-assistant:nextfrom
Bouni:current
Nov 11, 2018
Merged

Update Swiss Hydrological Data documentation#6535
fabaff merged 4 commits intohome-assistant:nextfrom
Bouni:current

Conversation

@Bouni
Copy link
Copy Markdown
Contributor

@Bouni Bouni commented Oct 5, 2018

Description:

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

Checklist:

  • Branch: next is for changes and new documentation that will go public with the next home-assistant release. Fixes, changes and adjustments for the current release should be created against current.
  • The documentation follow the standards.


<p class='note warning'>
This sensor doesn't work at the moment due to changed by the [Swiss Federal Office for the Environment (Bundesamt für Umwelt - Abt. Hydrologie)](http://www.hydrodaten.admin.ch) to access the data.
<p class='note info'>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

IIRC class='note' is enough.

- The mean discharge measurement of the last 24 hours

This sensor contains additional information which an easily accessed by a [template sensor](/components/sensor.template/).
<p class='note info'>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same

@frenck frenck added enhancement Hacktoberfest An PR on this issue (or the PR itself) is eligible towards Hacktoberfest! ready-for-review This PR needs to be reviewed current This PR goes into the current branch and removed to-do labels Oct 6, 2018
frenck
frenck previously requested changes Oct 6, 2018
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.

@Bouni Thank you for this PR. Some comments were made on it. Could you please take a look? 👍

description: list of measurements you want to use.
required: true
type: list
deafult:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Typo: default. Also, default should have a value.

required: true
type: list
deafult:
{% endconfiguration %}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is a list and that should have a keys attribute. You can use that attribute to list the possible measurement values.

Valid measurement values are:

## {% linkable_title Example %}
- temperature
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Move this section into the keys second of the measurements configuration value.

title: "Swiss Hydrological Data"
description: "Instructions on how to integrate hydrological data of Swiss waters within Home Assistant."
date: 2016-06-17 17:00
date: 2018-10-05 12:00
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please do not update the date. It marks the initial introduction.

@frenck frenck added in-progress This PR/Issue is currently being worked on and removed ready-for-review This PR needs to be reviewed labels Oct 6, 2018
@Bouni
Copy link
Copy Markdown
Contributor Author

Bouni commented Oct 8, 2018

I've got some feedback on the component itself. Therefore I will update fix the documentation as soon as I've found a solution for the addressed issues with the component!

@frenck
Copy link
Copy Markdown
Member

frenck commented Oct 8, 2018

Thanks for letting us know @Bouni!
We'll wait patiently 👍

@frenck frenck added new-feature This PR adds documentation for a new Home Assistant feature to an existing integration has-parent This PR has a parent PR in another repo needs-rebase The PR has been branched of the wrong base branch or targets an incorrect target branch and removed current This PR goes into the current branch enhancement labels Oct 8, 2018
@frenck
Copy link
Copy Markdown
Member

frenck commented Oct 8, 2018

Hold on sec, this PR depends on a parent PR.
This docs PR should go into the next branch.
@Bouni Please be sure to update the target and rebase your changes.

@Bouni
Copy link
Copy Markdown
Contributor Author

Bouni commented Oct 9, 2018

@frenck I've used current because the documentation says that updates of existing components should go into current, just documentation on new components should go into next.
Anyway, I'll do what you said 👍

@frenck
Copy link
Copy Markdown
Member

frenck commented Oct 9, 2018

@Bouni I understand the confusion.

Updates to the current release or documentation fixes go into the current branch. Changes that depend on a next release go into next. Since you are changing code (which results in changing docs), the change should go into next. That documentation will go live when the next release takes place.

This ensures the docs & release are in sync (and matches).

@fabaff fabaff changed the base branch from current to next November 11, 2018 15:57
@fabaff fabaff dismissed frenck’s stale review November 11, 2018 16:39

Comments addressed

Copy link
Copy Markdown
Member

@fabaff fabaff left a comment

Choose a reason for hiding this comment

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

🐦

@fabaff fabaff merged commit 01f8ca6 into home-assistant:next Nov 11, 2018
@ghost ghost removed the in-progress This PR/Issue is currently being worked on label Nov 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Hacktoberfest An PR on this issue (or the PR itself) is eligible towards Hacktoberfest! has-parent This PR has a parent PR in another repo needs-rebase The PR has been branched of the wrong base branch or targets an incorrect target branch new-feature This PR adds documentation for a new Home Assistant feature to an existing integration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants