-
Notifications
You must be signed in to change notification settings - Fork 1.8k
OBSDOCS-1612: Move the alertmanager configuration modules under a new… #86941
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
OBSDOCS-1612: Move the alertmanager configuration modules under a new… #86941
Conversation
@eromanova97: This pull request references OBSDOCS-1612 which is a valid jira issue. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
a207a24
to
20b0bb9
Compare
@eromanova97: This pull request references OBSDOCS-1612 which is a valid jira issue. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
20b0bb9
to
02f1800
Compare
@eromanova97: This pull request references OBSDOCS-1612 which is a valid jira issue. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
bd6b167
to
bc59b87
Compare
@eromanova97: This pull request references OBSDOCS-1612 which is a valid jira issue. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
bc59b87
to
5e7bfee
Compare
@eromanova97: This pull request references OBSDOCS-1612 which is a valid jira issue. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
modules/monitoring-configuring-alert-routing-default-platform-alerts.adoc
Outdated
Show resolved
Hide resolved
modules/monitoring-configuring-alert-routing-user-defined-alerts-secret.adoc
Outdated
Show resolved
Hide resolved
5e7bfee
to
7f5bbf3
Compare
/lgtm |
/label peer-review-needed |
@eromanova97: This pull request references OBSDOCS-1612 which is a valid jira issue. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
/remove-label peer-review-needed |
/label peer-review-needed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few comments; otherwise LGTM
modules/monitoring-enabling-alert-routing-for-user-defined-projects.adoc
Outdated
Show resolved
Hide resolved
...ty/monitoring/configuring-core-platform-monitoring/configuring-alerts-and-notifications.adoc
Outdated
Show resolved
Hide resolved
...onitoring/configuring-user-workload-monitoring/configuring-alerts-and-notifications-uwm.adoc
Outdated
Show resolved
Hide resolved
...onitoring/configuring-user-workload-monitoring/configuring-alerts-and-notifications-uwm.adoc
Outdated
Show resolved
Hide resolved
...onitoring/configuring-user-workload-monitoring/configuring-alerts-and-notifications-uwm.adoc
Outdated
Show resolved
Hide resolved
7f5bbf3
to
68b8f96
Compare
New changes are detected. LGTM label has been removed. |
/label merge-review-needed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking good! I found a couple tiny changes to make, and was thinking about the "before you begin" titles, but that last one is out of scope here and just for your consideration as you work on this.
/remove-label merge-review-in-progress
/remove-label merge-review-needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Musing on structure...
So, I like the idea of "before you begin" but I am not sure if it's right as the assembly title. It makes sense to me as an introduction in that section, but right now we have a page called "before you begin" that includes a bunch of actual procedures under it. It also shows in the left nav as "before you begin" - not sure a user would expect to see so many procedures when they click that in the nav.
Does it make sense to call this assembly something elese and bump the "before you begin" content into a first subsection? Maybe something about "preparing", which would imply the need to read it first but also match expectations of a lot of procedural work?
NB: it is also very possible I am just misunderstanding :)
modules/monitoring-configuring-alert-routing-default-platform-alerts.adoc
Outdated
Show resolved
Hide resolved
modules/monitoring-configuring-alert-routing-user-defined-alerts-secret.adoc
Outdated
Show resolved
Hide resolved
68b8f96
to
d0f4d4c
Compare
@eromanova97: all tests passed! Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
cff85a5
into
openshift:monitoring-docs-restructure
Version(s): none for cherry-picking,
monitoring-docs-restruce
onlyIssue: OBSDOCS-1612
Links to docs preview:
QE review:
Additional information:
The changes it this PR are not yet user-facing.
This PR adds an existing content to about alert configurations to the new assembly.
The content in the original assembly is still there . The reason is to not lose any content while moving it around. There will be a separate issue that will make sure that all the content is transfered as needed.
Therefore, the main thing to check in this PR is to see if the structure in the linked chapters looks good and renders without issues.
This PR also makes small changes to naming of procedures to mitigate the confusion about using multiple terms for the same configuration.
NOTE: The links and xrefs will be filled in in a different PR, because the content from other assemblies still needs to be moved to the new place, which means that if I filled the links now, I would have to keep repairing them later. So I am keeping that for a separate PR.