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

[Ready for Review] Reworking Rules docs #1860

Merged
merged 38 commits into from Nov 18, 2022

Conversation

rkoshak
Copy link
Contributor

@rkoshak rkoshak commented Jul 8, 2022

Getting started implementing #1855.

This first commit has some placeholders for the files we'll need but doesn't include any content changes yet.

Signed-off-by: Richard Koshak rlkoshak@gmail.com

Signed-off-by: Richard Koshak <rlkoshak@gmail.com>
@rkoshak rkoshak changed the title [WIP] Getting started [WIP] Reworking Rules docs Jul 8, 2022
@netlify
Copy link

netlify bot commented Jul 8, 2022

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).

Name Link
🔨 Latest commit c448d72
🔍 Latest deploy log https://app.netlify.com/sites/openhab-docs-preview/deploys/6378086d205ab30009fa2bbf
😎 Deploy Preview https://deploy-preview-1860--openhab-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

florian-h05 and others added 11 commits July 12, 2022 23:30
This covers 1 and 1a of the table at
openhab#1855.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
[Rules concepts] Add triggers & conditions

Looks great. Later on when we return to fix the references I might take out the list of cron fields. I'm not sure we need them here since we link to comprehensive docs on cron anyway and they feel a little out of place. But they do add some value so I'm not sure. 

Beyond that it looks great! Onward to the next steps!
@Confectrician Confectrician added this to the 3.4 milestone Sep 10, 2022
@Confectrician
Copy link
Contributor

Hey @rkoshak,

i know this is a big one and you have many things on your list usually.
Is there any chance that we split the changes/to-do's from #1855 and do a partly merge?

From my view any refactored documentation is a win for us, even if it isn't 100% and includes all asked points. :)
There are already good improvements here, that are laying around for weeks now.

Is a partly change possible or would that have a negetaive effect for your workigns on this?

@rkoshak
Copy link
Contributor Author

rkoshak commented Sep 11, 2022

We can but even what we have now isn't really ready. If at least like to get the concepts page done before the first merge. Then I think we can treat each page after that as a separate PR. Though if like to continue to track the progress on the same issue if that's ok.

@florian-h05
Copy link
Contributor

florian-h05 commented Sep 12, 2022

I agree on @rkoshak ‘s opinion.
Let’s get the concepts page done, and then have the first merge.

FYI: I continue to work on the concepts page, but I will probably leave some parts out, e.g. the rule templates as this is clearly a thing @rkoshak is far more experienced in.

I read the community thread you linked, a scene rule is definitely something that has to be included in the examples of the concepts page.
I am also thinking of saying that it is possible to realise scenes with rules on top of the page.

@openhab-bot
Copy link
Collaborator

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/scenes-openhab-vs-home-assistant-in-2022/138782/21

@Confectrician
Copy link
Contributor

Though if like to continue to track the progress on the same issue if that's ok.

Yes of course. :)

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
…es sections

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
…ippets

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
@florian-h05
Copy link
Contributor

@Confectrician
FYI: The concepts page should be finished soon, and can then be merged.

Rkoshak reviewed my contribution of the concepts page, so there shouldn‘t be need for a super detailed review from your side, I think there are just some styling issues (e.g. the ToC) where we need your help regarding Jekyll.

rkoshak and others added 12 commits November 3, 2022 09:41
Added the new rules concepts page to the sidebar.

Signed-off-by: Rich Koshak <rlkoshak@gmail.com>
Signed-off-by: Richard Koshak <rlkoshak@gmail.com>
Signed-off-by: Richard Koshak <rlkoshak@gmail.com>
Signed-off-by: Richard Koshak <rlkoshak@gmail.com>
Signed-off-by: Richard Koshak <rlkoshak@gmail.com>
Signed-off-by: Richard Koshak <rlkoshak@gmail.com>
Signed-off-by: Richard Koshak <rlkoshak@gmail.com>
Signed-off-by: Richard Koshak <rlkoshak@gmail.com>
Signed-off-by: Richard Koshak <rlkoshak@gmail.com>
Signed-off-by: Richard Koshak <rlkoshak@gmail.com>
@rkoshak
Copy link
Contributor Author

rkoshak commented Nov 3, 2022

~~I think I broke it. ~~

~~I've been trying to fix the premerge checks like linting and such and now it's not running those checks nor is it deploying the preview. I'm not sure what I can do at this point. ~~

Ha! Renaming the PR seems to kicked it off.

@rkoshak rkoshak changed the title [WIP] Reworking Rules docs [Ready for Review] Reworking Rules docs Nov 3, 2022
Signed-off-by: Richard Koshak <rlkoshak@gmail.com>
Signed-off-by: Richard Koshak <rlkoshak@gmail.com>
@rkoshak
Copy link
Contributor Author

rkoshak commented Nov 3, 2022

@Confectrician , this is ready for review.

None of the other concepts pages have a TOC but I think it's useful to have it for the rules page. Should we be consistent and remove it (or add one to the other pages) or keep it in even if it's inconsistent? Also, is there a way to set the TOC to only show level 2 headings? I don't think we need to go to level 3 and 4 for this page. I tried changing the levels from 2..4 to just 2 but it's still showing them all.

The MD linting errors are not from our changes (and frankly I think they are false positives).

@florian-h05, It seems we cannot have a space in the title for the tabs in our examples. So I renamed the tabs to UI, DSL, JRuby, and JS.

Also, now that I see it, I wonder if later on we ought to replace the JS examples with rule builder. As it is right now, the JS examples look really wordy compared to even Rules DSL. We can leave it as is for this PR though but I think the JS examples will look more comparable using the builder instead of JSRule.

Finally, @Confectrician, the next PR will create a new "Rules" section in the sidebar (similar to the UIs section). Is there anything else I need to modify beyond the .vuepress/config.js file? It seems I remember needing to edit another file too but I can't figure out what.

@Confectrician
Copy link
Contributor

@Confectrician , this is ready for review.

🎉

None of the other concepts pages have a TOC but I think it's useful to have it for the rules page. Should we be consistent and remove it (or add one to the other pages) or keep it in even if it's inconsistent? Also, is there a way to set the TOC to only show level 2 headings? I don't think we need to go to level 3 and 4 for this page. I tried changing the levels from 2..4 to just 2 but it's still showing them all.

If you refer to the curly braced configuration you can throw it away anyways.
This is an old way of configuring the TOC, we used when the docs were served via guthub pages.
From what i read we can configure the toc only for the complete website.

The MD linting errors are not from our changes (and frankly I think they are false positives).

They are correct, but i am already working on #1906 and the should be solved then.

I will need some time for review, but would try to get it done over the weekend.

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.

Sorry for the delay.
Finally had some time to check this.
Just som minor things i will solve myself.

concepts/rules.md Outdated Show resolved Hide resolved
concepts/rules.md Outdated Show resolved Hide resolved
Signed-off-by: Jerome Luckenbach <github@luckenba.ch>
@Confectrician Confectrician merged commit e92b820 into openhab:main Nov 18, 2022
@rkoshak rkoshak deleted the rules-rework branch November 18, 2022 22:54
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.

None yet

4 participants