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

Adding content for the new Notifications plugin #586

Merged
merged 10 commits into from
May 26, 2022
Merged

Conversation

keithhc2
Copy link
Contributor

@keithhc2 keithhc2 commented May 18, 2022

Adds the Notifications plugin

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Site URLs:

../ism/policies.md
https://opensearch.org/docs/latest/im-plugin/ism/policies/

_monitoring-plugins/alerting/monitors.md
https://opensearch.org/docs/latest/monitoring-plugins/alerting/monitors/

(new pages/not yet on site):
_notifications-plugin/api.md
_notifications-plugin/index.md

@keithhc2 keithhc2 added upcoming release Don't merge until the version or feature is available v2.0.0 labels May 18, 2022
@keithhc2 keithhc2 requested a review from a team as a code owner May 18, 2022 18:39
@alicejw1 alicejw1 self-requested a review May 18, 2022 18:42
Signed-off-by: keithhc2 <[email protected]>
Copy link
Contributor

@alicejw1 alicejw1 left a comment

Choose a reason for hiding this comment

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

Really great work Keith!
i made some small suggestions

LGTM

Signed-off-by: keithhc2 <[email protected]>
Copy link
Contributor

@alicejw1 alicejw1 left a comment

Choose a reason for hiding this comment

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

I made some suggestions for the API descriptions.

Copy link
Collaborator

@natebower natebower left a comment

Choose a reason for hiding this comment

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

Please see my suggested changes and let me know if you have any questions. Thanks!

_notifications-plugin/api.md Outdated Show resolved Hide resolved
_monitoring-plugins/alerting/monitors.md Outdated Show resolved Hide resolved
_notifications-plugin/api.md Outdated Show resolved Hide resolved
_notifications-plugin/api.md Outdated Show resolved Hide resolved
_notifications-plugin/api.md Outdated Show resolved Hide resolved
_notifications-plugin/index.md Outdated Show resolved Hide resolved
_notifications-plugin/index.md Outdated Show resolved Hide resolved
_notifications-plugin/index.md Outdated Show resolved Hide resolved
_notifications-plugin/index.md Outdated Show resolved Hide resolved
_notifications-plugin/index.md Outdated Show resolved Hide resolved
Copy link
Contributor

@alicejw1 alicejw1 left a comment

Choose a reason for hiding this comment

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

Hi Keith, i had to modify my comments to be consistent. I'm finished with the review now.
thanks,
Alice

Copy link
Contributor

@alicejw1 alicejw1 left a comment

Choose a reason for hiding this comment

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

Hi Keith, i needed to update my comment to make it more specific.
Thanks,
Alice

_monitoring-plugins/alerting/monitors.md Show resolved Hide resolved
_notifications-plugin/api.md Outdated Show resolved Hide resolved
_notifications-plugin/api.md Outdated Show resolved Hide resolved
_notifications-plugin/index.md Outdated Show resolved Hide resolved
_notifications-plugin/index.md Outdated Show resolved Hide resolved
_notifications-plugin/index.md Outdated Show resolved Hide resolved
_notifications-plugin/index.md Outdated Show resolved Hide resolved
_monitoring-plugins/alerting/monitors.md Show resolved Hide resolved
@natebower natebower removed the 5 - Editorial review PR: Editorial review in progress label May 23, 2022
Signed-off-by: keithhc2 <[email protected]>
Signed-off-by: keithhc2 <[email protected]>
@keithhc2 keithhc2 requested review from lizsnyder and Naarcha-AWS May 24, 2022 16:20
_notifications-plugin/api.md Outdated Show resolved Hide resolved
_notifications-plugin/api.md Outdated Show resolved Hide resolved
Copy link
Contributor

@hdhalter hdhalter left a comment

Choose a reason for hiding this comment

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

Nice job, Alice! I just have a few minor tweak suggestions.

_monitoring-plugins/alerting/monitors.md Outdated Show resolved Hide resolved
_notifications-plugin/index.md Outdated Show resolved Hide resolved
_notifications-plugin/index.md Outdated Show resolved Hide resolved
_notifications-plugin/index.md Outdated Show resolved Hide resolved
_monitoring-plugins/alerting/monitors.md Show resolved Hide resolved
keithhc2 added 2 commits May 24, 2022 16:34
@hdhalter
Copy link
Contributor

@keithhc2 - I'd be happy to sign off on this, but I don't see the option. Do you have to add me?

@keithhc2 keithhc2 requested a review from hdhalter May 25, 2022 16:21
@keithhc2
Copy link
Contributor Author

keithhc2 commented May 25, 2022

@hdhalter If you go to the Files changed tab, you should see a green Review changes button. Click that, and you can comment/approve/request changes/etc.

image

@hdhalter
Copy link
Contributor

Screen Shot 2022-05-25 at 9 25 57 AM

Signed-off-by: keithhc2 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
upcoming release Don't merge until the version or feature is available v2.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Documentation - Support Notifications and Alerts in 2.0.0 (Breaking Changes)
6 participants