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

[Alerts][Docs] Extended README.md and the user docs with the licensing information. #92564

Merged
merged 3 commits into from
Feb 25, 2021

Conversation

YulNaumenko
Copy link
Contributor

@YulNaumenko YulNaumenko commented Feb 24, 2021

Should provide the missing alerts licensing information added by the PR

  • 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 docs v7.12.0 labels Feb 24, 2021
@YulNaumenko YulNaumenko requested a review from gchaps February 24, 2021 05:48
@YulNaumenko YulNaumenko requested a review from a team as a code owner February 24, 2021 05:48
@YulNaumenko YulNaumenko self-assigned this Feb 24, 2021
Co-authored-by: Lisa Cawley <[email protected]>
Co-authored-by: gchaps <[email protected]>
@YulNaumenko YulNaumenko requested a review from lcawl February 24, 2021 21:40
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

Comment on lines 136 to 137
You should create documentation for the new alert type. Make an entry in the alert type index [`docs/user/alerting/alert-types.asciidoc`](../../../docs/user/alerting/alert-types.asciidoc) that points to a new document for the alert type that should be in the directory [`docs/user/alerting/alert-types`](../../../docs/user/alerting/alert-types).

Copy link
Contributor

Choose a reason for hiding this comment

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

Presumably solution specific alert types should be under their directories rather than docs/user/alerting/alert-types/? 🤔

If so, best to clarify that

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 replace this part with more generic definition. 👍

Copy link
Contributor

@gmmorris gmmorris left a comment

Choose a reason for hiding this comment

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

LGTM, but I left a couple of notes worth considering.


## Tests

The alert type should have both jest tests and functional tests.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it worth clarifying what should be tested (such as alert specific action groups, parameters etc.) vs. things that we guarantee as a framework and do not need to be independently tested by each alert type? 🤔

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

cc @YulNaumenko

@YulNaumenko YulNaumenko merged commit 0627573 into elastic:master Feb 25, 2021
YulNaumenko added a commit to YulNaumenko/kibana that referenced this pull request Feb 25, 2021
…g information. (elastic#92564)

* [Alerts][Docs] Extended README.md and the user docs with the licensing information.

* Apply suggestions from code review

Co-authored-by: Lisa Cawley <[email protected]>
Co-authored-by: gchaps <[email protected]>

* fixed due to comments

Co-authored-by: Lisa Cawley <[email protected]>
Co-authored-by: gchaps <[email protected]>
YulNaumenko added a commit to YulNaumenko/kibana that referenced this pull request Feb 25, 2021
…g information. (elastic#92564)

* [Alerts][Docs] Extended README.md and the user docs with the licensing information.

* Apply suggestions from code review

Co-authored-by: Lisa Cawley <[email protected]>
Co-authored-by: gchaps <[email protected]>

* fixed due to comments

Co-authored-by: Lisa Cawley <[email protected]>
Co-authored-by: gchaps <[email protected]>
YulNaumenko added a commit that referenced this pull request Feb 25, 2021
…g information. (#92564) (#92907)

* [Alerts][Docs] Extended README.md and the user docs with the licensing information.

* Apply suggestions from code review

Co-authored-by: Lisa Cawley <[email protected]>
Co-authored-by: gchaps <[email protected]>

* fixed due to comments

Co-authored-by: Lisa Cawley <[email protected]>
Co-authored-by: gchaps <[email protected]>

Co-authored-by: Lisa Cawley <[email protected]>
Co-authored-by: gchaps <[email protected]>
YulNaumenko added a commit that referenced this pull request Feb 25, 2021
…g information. (#92564) (#92908)

* [Alerts][Docs] Extended README.md and the user docs with the licensing information.

* Apply suggestions from code review

Co-authored-by: Lisa Cawley <[email protected]>
Co-authored-by: gchaps <[email protected]>

* fixed due to comments

Co-authored-by: Lisa Cawley <[email protected]>
Co-authored-by: gchaps <[email protected]>

Co-authored-by: Lisa Cawley <[email protected]>
Co-authored-by: gchaps <[email protected]>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Feb 26, 2021
…bana into task-manager/docs-monitoring

* 'task-manager/docs-monitoring' of github.com:gmmorris/kibana: (40 commits)
  [Security Solution][Case][Bug] Improve case logging (elastic#91924)
  [Alerts][Doc] Added README documentation for alerts plugin status and framework health checks configuration options. (elastic#92761)
  Add warning for EQL and Threshold rules if exception list contains value list items (elastic#92914)
  [Security Solution][Case] Fix subcases bugs on detections and case view (elastic#91836)
  [APM] Always allow access to Profiling via URL (elastic#92889)
  [Vega] Allow image loading without CORS policy by changing the default to crossOrigin=null (elastic#91991)
  skip flaky suite (elastic#92114)
  [APM] Fix for default fields in correlations view (elastic#91868) (elastic#92090)
  chore(NA): bump bazelisk to v1.7.5 (elastic#92905)
  [Maps] fix selecting EMS basemap does not populate input (elastic#92711)
  API docs (elastic#92827)
  [kbn/test] add import/export support to KbnClient (elastic#92526)
  Test fix management scripted field filter functional test and unskip it  (elastic#92756)
  [App Search] Create Curation view/functionality (elastic#92560)
  [Reporting/Discover] include the document's entire set of fields (elastic#92730)
  [Fleet] Add new index to fleet for artifacts being served out of fleet-server (elastic#92860)
  [Alerts][Doc] Added README documentation for API key invalidation configuration options. (elastic#92757)
  [Discover][docs] Add search for relevance (elastic#90611)
  [Alerts][Docs] Extended README.md and the user docs with the licensing information. (elastic#92564)
  [7.12][Telemetry] Security telemetry allowlist fix. (elastic#92850)
  ...
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 v7.12.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants