Skip to content

Fix merge error & added doc for sensehat new flag #2100

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

Merged
merged 2 commits into from
Feb 23, 2017

Conversation

nodinosaur
Copy link
Contributor

@nodinosaur nodinosaur commented Feb 20, 2017

Description:
Added a new Flag for if the Sensehat is Attached to the R-Pi

Fixed a messed up merge

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

@mention-bot
Copy link

@nodinosaur, thanks for your PR! By analyzing the history of the files in this pull request, we identified @fabaff to be a potential reviewer.

@@ -26,6 +26,7 @@ sensor:
- temperature
- humidity
- pressure
is_hat_attached: True|False
Copy link
Member

Choose a reason for hiding this comment

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

It's an optional entry. Thus don't add it. We try to keep the configuration samples minimal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure

@@ -34,56 +35,13 @@ Configuration variables:
- 'temperature'
- 'humidity'
- 'pressure'
is_hat_attached (Optional): True|False boolean; True declares the SenseHAT _is_ physically on the Raspberry Pi
Copy link
Member

Choose a reason for hiding this comment

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

Please state what the default is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure


### {% linkable_title Installation instruction for the All-In-One installer and HASSbian %}
#### Customizing the Sense HAT data
Copy link
Member

Choose a reason for hiding this comment

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

This part was already added by #2009. I suggest that you limit your PR to the new Boolean and make it against next which is the right branch for new features.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The previous issue has a styling issue when merge occurred. This corrects that on top of that PR. This is the correct version.

Copy link
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.

Thanks 🐦

@fabaff fabaff merged commit e8aafa6 into home-assistant:current Feb 23, 2017
@nodinosaur nodinosaur deleted the patch-3 branch February 23, 2017 13:20
fabaff pushed a commit that referenced this pull request Feb 23, 2017
* Fix merge error & added doc for sensehat new flag

* Changes as per PR comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants