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

[Alerting][Docs] Moved alerting links from hard-coded to documentation link service. #92953

Merged

Conversation

YulNaumenko
Copy link
Contributor

@YulNaumenko YulNaumenko commented Feb 26, 2021

Resolves #88231

  • Documentation was added for features that require explanation or tutorials

@YulNaumenko YulNaumenko added Feature:Alerting v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) docs v7.12.0 labels Feb 26, 2021
@YulNaumenko YulNaumenko requested a review from a team as a code owner February 26, 2021 05:50
@YulNaumenko YulNaumenko self-assigned this Feb 26, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

Copy link
Contributor

@ymao1 ymao1 left a comment

Choose a reason for hiding this comment

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

LGTM! Just one comment about updating the links to the stack alerts pages.

@@ -56,7 +56,7 @@ export const EmailActionConnectorFields: React.FunctionComponent<
)}
helpText={
<EuiLink
href={`${docLinks.ELASTIC_WEBSITE_URL}guide/en/kibana/${docLinks.DOC_LINK_VERSION}/email-action-type.html#configuring-email`}
href={`${docLinks.links.alerting.emailAction}#configuring-email`}
Copy link
Contributor

Choose a reason for hiding this comment

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

href={${docLinks.links.alerting.emailAction}#configuring-email}

I think the downside of putting the section anchor here instead of adding a new keyword in the doc link service, is that if the section is removed or changed, our link checking won't catch it. I think in cases where the section no longer exists, the link will default to the top of the page. If that's acceptable, then this is an okay implementation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will change it to have an own link - I think it's better to maintain 👍

@mikecote mikecote self-requested a review March 1, 2021 19:40
Copy link
Contributor

@mikecote mikecote left a comment

Choose a reason for hiding this comment

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

Changes LGTM!

@YulNaumenko YulNaumenko requested a review from a team as a code owner March 1, 2021 22:08
…ts/health_check.tsx

Co-authored-by: Mike Côté <mikecote@users.noreply.github.com>
@YulNaumenko YulNaumenko requested a review from lcawl March 1, 2021 22:09
Copy link
Contributor

@TinaHeiligers TinaHeiligers left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@lcawl lcawl left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
stackAlerts 682.7KB 682.6KB -80.0B
triggersActionsUi 1.6MB 1.5MB -25.9KB
total -25.9KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
core 479.8KB 480.1KB +286.0B
stackAlerts 19.2KB 19.0KB -180.0B
triggersActionsUi 104.0KB 104.1KB +82.0B
total +188.0B
Unknown metric groups

async chunk count

id before after diff
triggersActionsUi 41 42 +1

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @YulNaumenko

@YulNaumenko YulNaumenko merged commit a102fa9 into elastic:master Mar 2, 2021
YulNaumenko added a commit to YulNaumenko/kibana that referenced this pull request Mar 2, 2021
…n link service. (elastic#92953)

* [Alerting][Docs] Moved alerting links from hard-coded to documentation link service

* fixed due to comments

* Update x-pack/plugins/triggers_actions_ui/public/application/components/health_check.tsx

Co-authored-by: Mike Côté <mikecote@users.noreply.github.com>

* fixed jest tests

* fixed due to comments

Co-authored-by: Mike Côté <mikecote@users.noreply.github.com>
YulNaumenko added a commit to YulNaumenko/kibana that referenced this pull request Mar 2, 2021
…n link service. (elastic#92953)

* [Alerting][Docs] Moved alerting links from hard-coded to documentation link service

* fixed due to comments

* Update x-pack/plugins/triggers_actions_ui/public/application/components/health_check.tsx

Co-authored-by: Mike Côté <mikecote@users.noreply.github.com>

* fixed jest tests

* fixed due to comments

Co-authored-by: Mike Côté <mikecote@users.noreply.github.com>
YulNaumenko added a commit that referenced this pull request Mar 3, 2021
…n link service. (#92953) (#93321)

* [Alerting][Docs] Moved alerting links from hard-coded to documentation link service

* fixed due to comments

* Update x-pack/plugins/triggers_actions_ui/public/application/components/health_check.tsx

Co-authored-by: Mike Côté <mikecote@users.noreply.github.com>

* fixed jest tests

* fixed due to comments

Co-authored-by: Mike Côté <mikecote@users.noreply.github.com>

Co-authored-by: Mike Côté <mikecote@users.noreply.github.com>
YulNaumenko added a commit that referenced this pull request Mar 3, 2021
…n link service. (#92953) (#93322)

* [Alerting][Docs] Moved alerting links from hard-coded to documentation link service

* fixed due to comments

* Update x-pack/plugins/triggers_actions_ui/public/application/components/health_check.tsx

Co-authored-by: Mike Côté <mikecote@users.noreply.github.com>

* fixed jest tests

* fixed due to comments

Co-authored-by: Mike Côté <mikecote@users.noreply.github.com>

Co-authored-by: Mike Côté <mikecote@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Feature:Alerting release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.12.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move alerting links from hard-coded to documentation link service
7 participants