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

[openhabcloud] Update README.md #5425

Closed
wants to merge 1 commit into from
Closed

Conversation

marcopelly
Copy link

Explicitly added the need to install the add-on of openHAB Cloud connector

Explicitly added the need to install the add-on of openHAB Cloud connector
@marcopelly marcopelly requested a review from kaikreuzer as a code owner April 7, 2019 14:26
@wborn
Copy link
Member

wborn commented Apr 7, 2019

I agree it would help to instruct how to install an add-on. But this info applies to every add-on and it could also be generated. See openhab/website#152.

@davidgraeff
Copy link
Member

The change is just one sentence that indeed helps in this context. Can you please split up by sentences? In future reviews it is than more obvious if just one sentence got changed or added.

@wborn
Copy link
Member

wborn commented Apr 7, 2019

There seems to be common agreement add-on installation should not be part of every add-on documentation. See https://github.com/openhab/openhab2-addons/pull/3162.

@davidgraeff
Copy link
Member

I see, in this case we cannot merge. @wborn can you please stop calling specific patterns "common agreement" because neither me nor J-N-K nor any other new maintainer agreed on those or even know about those pattern and it was just you and Kai actually.

@wborn
Copy link
Member

wborn commented Apr 7, 2019

OK let's call it something else. But if this gets merged, one day a similar PR for every other add-on might also get merged for the same reasons.

@J-N-K
Copy link
Member

J-N-K commented Apr 7, 2019

In general I agree with @wborn. But we should get the documentation update. After reading the issues linked above it is not clear to me what is missing and how to proceed.

@Hilbrand
Copy link
Member

Hilbrand commented Apr 7, 2019

This isn't about adding installation. But the fact that the binding triggers the generation of the keys. This isn't clear from the current documentation as it refers to the generic term openHAB runtime. So it makes sense to change something. But I think the first sentence can be changed in: To authenticate with the openHAB Cloud the openHAB cloud add-on generates two values when the add-on is installed, which need to be entered in your account settings of the openHAB Cloud service.

@J-N-K J-N-K changed the title Update README.md [openhabcloud] Update README.md Apr 7, 2019
@davidgraeff
Copy link
Member

A new PR is very welcome.
Please remember that the documentation team like to have one sentence per line in readme.md files.

Closing this now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants