Skip to content

Rename alertTypeParams to params#51255

Merged
mikecote merged 6 commits intoelastic:masterfrom
mikecote:alerting/rename-alert-type-params
Nov 25, 2019
Merged

Rename alertTypeParams to params#51255
mikecote merged 6 commits intoelastic:masterfrom
mikecote:alerting/rename-alert-type-params

Conversation

@mikecote
Copy link
Contributor

In this PR, I'm renaming alertTypeParams to params for consistency with the attribute names within actions.

Fixes #49704.

cc @YulNaumenko as the rename will also have to change within the UI branch.
cc @FrankHassanabad as a heads up of a breaking change for SIEM PRs not yet merged. I renamed what I could of what's within master.

@mikecote mikecote requested a review from a team November 21, 2019 04:17
@mikecote mikecote self-assigned this Nov 21, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-stack-services (Team:Stack Services)

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

Copy link
Contributor

@pmuellr pmuellr left a comment

Choose a reason for hiding this comment

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

LGTM

I think we should probably keep a "short list" somewhere of our current users - this will be a breaking change for them so we should give them a send-ahead.

This will also require a change to kbn-alert in https://github.com/pmuellr/kbn-action

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@mikecote
Copy link
Contributor Author

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@mikecote mikecote requested a review from gmmorris November 22, 2019 20:24
@mikecote
Copy link
Contributor Author

cc @elastic/stack-monitoring & @elastic/siem upcoming breaking change in alerting.

Copy link
Contributor

@FrankHassanabad FrankHassanabad left a comment

Choose a reason for hiding this comment

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

Checked this out and tested it with ad-hoc e2d tests where I posted some signals, deleted them, and did find against them and everything looks ok.

Really appreciate you looking out for us @mikecote and making these changes in our code base and pinging us.

👍 LGTM

@mikecote
Copy link
Contributor Author

@elasticmachine merge upstream

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

Nothing meaningful to give feedback on as it's mainly a rename and it seems to still work, so 👍

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

mikecote added a commit to mikecote/kibana that referenced this pull request Nov 25, 2019
chrisronline added a commit to chrisronline/kibana that referenced this pull request Nov 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:Alerting release_note:skip Skip the PR/issue when compiling release notes v7.6.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rename alert attribute "alertTypeParams" to "params"

5 participants