-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[FTR][Ownership] add @elastic/security-detection-engine to es archives #193571
Conversation
@elasticmachine merge upstream |
.github/CODEOWNERS
Outdated
@@ -1678,6 +1678,8 @@ x-pack/test/security_solution_cypress/cypress/tasks/expandable_flyout @elastic/ | |||
/x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine @elastic/security-detection-engine | |||
/x-pack/test/security_solution_api_integration/test_suites/detections_response/utils/rules/rule_gaps.ts @elastic/security-detection-engine | |||
/x-pack/test/security_solution_api_integration/test_suites/lists_and_exception_lists @elastic/security-detection-engine | |||
/x-pack/test/functional/es_archives/asset_criticality @elastic/security-detection-engine | |||
/x-pack/test/functional/es_archives/security_solution @elastic/security-detection-engine |
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.
I don't think that /x-pack/test/functional/es_archives/security_solution - all archives should be owned as detection engine team
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.
ah ok, so then it's divided from there right?
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.
there a lot of archives inside this folder that can be used by several teams.
I think's its ok for this PR to only keep asset_criticality for DE
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.
Ok, makes sense. I feel a kibana-wide email coming on then 😉
I'll keep the pr open for now, but back in draft.
First I'll look around some more for your team specifically.
Then I get the feeling that email will help a ton.
Drop line with discrepancy for now.
@elasticmachine merge upstream |
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]
History
To update your PR or re-run it, just comment with: |
Summary
Modify code owner declarations for @elastic/security-detection-engine team in
.github/CODEOWNERS
Update for Reviewers
With the merge of this, you can now do this:
node scripts/get_owners_for_file.js --file x-pack/test/functional/es_archives/asset_criticality
So, you can use this script to see if the code owners file is reporting erroneously
Only one un-contested change:
/x-pack/test/functional/es_archives/asset_criticality @elastic/security-detection-engine
Note
Oringinally I included
/x-pack/test/functional/es_archives/security_solution @elastic/security-detection-engine
but this was contested.We will circle back around to this.
Contributes to: #192979