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 concepts] Add triggers & conditions #4

Merged
merged 5 commits into from
Aug 25, 2022

Conversation

florian-h05
Copy link

@florian-h05 florian-h05 commented Aug 17, 2022

@rkoshak
Unfortunately I have bad weather today on my vacation trip, so this is the next microstep in reworking the rule documentation.

This PR should complete part 1b and 1c of openhab#1855.

Copy link
Owner

@rkoshak rkoshak left a comment

Choose a reason for hiding this comment

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

Over all it looks really good. Thanks for submitting. Most of these comments are nit picking but I think they are important.

rules/concepts.md Outdated Show resolved Hide resolved
rules/concepts.md Show resolved Hide resolved
rules/concepts.md Outdated Show resolved Hide resolved
rules/concepts.md Outdated Show resolved Hide resolved
rules/concepts.md Outdated Show resolved Hide resolved
rules/concepts.md Outdated Show resolved Hide resolved
rules/concepts.md Show resolved Hide resolved
rules/concepts.md Outdated Show resolved Hide resolved
rules/concepts.md Outdated Show resolved Hide resolved
rules/concepts.md Show resolved Hide resolved
@florian-h05
Copy link
Author

@rkoshak
Ready for re-review.
Out of interest: Do you get a notification when I re-request a review?

Copy link
Owner

@rkoshak rkoshak left a comment

Choose a reason for hiding this comment

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

Looks great! I've just a few typos and word order type comments and we are good to merge. Thanks again for working on this. I keep trying to find an hour where I can sit in front of a computer to type something out but it's not worked out yet.

I do get an alert through the Github app on my phone when it's a PR or issue I'm subscribed to or am mentioned in a comment. I also check the list of issues and PRs in my github notifications every couple of days. But I like to dedicate a contiguous time to do reviews like this which can delay my response.

rules/concepts.md Outdated Show resolved Hide resolved
rules/concepts.md Outdated Show resolved Hide resolved
rules/concepts.md Outdated Show resolved Hide resolved
rules/concepts.md Outdated Show resolved Hide resolved
rules/concepts.md Outdated Show resolved Hide resolved
rules/concepts.md Outdated Show resolved Hide resolved
Signed-off-by: Florian Hotze <[email protected]>
@florian-h05
Copy link
Author

Thanks again for working on this. I keep trying to find an hour where I can sit in front of a computer to type something out but it's not worked out yet.

You are welcome. I know the problem with the time, luckily I have school holidays and some time to work on this.

@rkoshak rkoshak merged commit 8d3ff3d into rkoshak:rules-rework Aug 25, 2022
@florian-h05 florian-h05 deleted the rules-rework-1 branch August 25, 2022 19:43
@florian-h05
Copy link
Author

Can you please update the issue and tick the checkboxes for 1b and 1c?

@rkoshak
Copy link
Owner

rkoshak commented Aug 25, 2022

Yes, already have it open, just got distracted for a bit.

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.

2 participants