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

Rules: Replace placeholder with ESH text #893

Merged
merged 5 commits into from
Feb 15, 2019

Conversation

davidgraeff
Copy link
Member

This is a first step in improving the NG Rules page.
In a follow up PR this is split up into a technical part and a user part and https://community.openhab.org/t/experimental-next-gen-rules-engine-documentation-2-of-installation/55955 need to be integrated of course.

Signed-off-by: davidgraeff [email protected]

This is a first step in improving the NG Rules page.
In a follow up PR this is split up into a technical part and a user part and https://community.openhab.org/t/experimental-next-gen-rules-engine-documentation-2-of-installation/55955 need to be integrated of course.

Signed-off-by: davidgraeff <[email protected]>
@netlify
Copy link

netlify bot commented Feb 7, 2019

Thanks for your pull request to the openHAB documentation! The result can be previewed at the URL below (this comment and the preview will be updated if you add more commits).

Built with commit baf4384

https://deploy-preview-893--openhab-docs-preview.netlify.com

Fix http->https
@Confectrician
Copy link
Contributor

Note for myself:

First of all we should check the sidebar config.
The page is located under the configuration folder but in the sidebar it is displayed under interfaces and ecosystem.

@5iver
Copy link
Contributor

5iver commented Feb 13, 2019

TMK, we haven't been given the go ahead to remove 'experimental'. IIRC, this is to be removed in OH3, and at this point, we can't be sure that will be the next release. There are also a lot of changes yet to come to the NGRE, so IMO it is probably best to keep that label to set expectations in functionality and the potential for breaking changes.

Copy link
Contributor

@Confectrician Confectrician left a comment

Choose a reason for hiding this comment

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

Hey @davidgraeff ,

Sorry for the delay.
I had some time for a first look.
Because of the amount i think i will have to look at the contribution again.

Here are my first comments.
Most of them are markdown related.

BR
Jerome

configuration/rules-ng.md Outdated Show resolved Hide resolved
configuration/rules-ng.md Outdated Show resolved Hide resolved
configuration/rules-ng.md Outdated Show resolved Hide resolved
configuration/rules-ng.md Outdated Show resolved Hide resolved
configuration/rules-ng.md Outdated Show resolved Hide resolved
configuration/rules-ng.md Show resolved Hide resolved
configuration/rules-ng.md Show resolved Hide resolved
configuration/rules-ng.md Show resolved Hide resolved
configuration/rules-ng.md Show resolved Hide resolved
configuration/rules-ng.md Show resolved Hide resolved
@davidgraeff
Copy link
Member Author

davidgraeff commented Feb 13, 2019

@openhab-5iver I have added the "Experimental" badge, which looks more professional IMO. But can revert that of course.

Also: If we are fading out DSL Rules, so recommending Jython/Javascript, this module will always be installed. Maybe we should replace "Experimental" with "Not-yet-for-end-users" or so.

@Confectrician
Copy link
Contributor

Hey @openhab-5iver ,

We could also edit the sidebar and add an (Experimental).
Anyways David has left the disclaimer/note at the beginning of the article so we should be safe for now.

This PR will be a huge improvement to the ngre documentation and a good base for imrpoving when we get changes or new functionality.

BR
Jerome

@5iver
Copy link
Contributor

5iver commented Feb 13, 2019

I have added the "Experimental" badge, which looks more professional IMO. But can revert that of course.

Oh, sorry. I missed that! It does look better and more professional.

Also: If we are fading out DSL Rules, so recommending Jython/Javascript, this module will always be installed. Maybe we should replace "Experimental" with "Not-yet-for-end-users" or so.

Actually, I've wondered whether all rule engines shouldn't be made optional. IMO, it can be labelled with anything that keeps people from thinking it's mainstream, or the only rule engine, or ready for for use. But we've been using 'experimental', so probably shouldn't change it.

We could also edit the sidebar and add an (Experimental).

Adding to the sidebar would be another helpful alert for people too!

Confectrician and others added 2 commits February 14, 2019 00:18
This is no longer a pure copy. I have broken down some super long sentences and reformulated some paragraphs.
Incorporate suggested changes and fixed typo
@davidgraeff
Copy link
Member Author

This is the technical description of the rules system however. It need to be split into a "technical concepts + details" page and a second page that is more user-centric. I will do that in a follow up PR after you guys sorted where to put this page here in the sidebar navigation. I suggest the development section as it is very technical.

Copy link
Contributor

@Confectrician Confectrician left a comment

Choose a reason for hiding this comment

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

Thanks @davidgraeff,

Everything adressed and already a huge improvement.

@Confectrician Confectrician merged commit cd9c81d into openhab:master Feb 15, 2019
@davidgraeff davidgraeff deleted the patch-1 branch February 15, 2019 21:47
@Confectrician Confectrician added this to the 2.x.x milestone Nov 18, 2022
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