-
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
[CODEOWNERS] fix appex-qa ownership #189602
Conversation
@@ -1273,7 +1269,7 @@ x-pack/test/observability_ai_assistant_functional @elastic/obs-ai-assistant | |||
/x-pack/test_serverless/api_integration/test_suites/common/elasticsearch_api @elastic/appex-qa | |||
/x-pack/test_serverless/functional/test_suites/security/ftr/ @elastic/appex-qa | |||
/x-pack/test_serverless/functional/test_suites/common/home_page/ @elastic/appex-qa | |||
/x-pack/test_serverless/functional/services/ @elastic/appex-qa | |||
/x-pack/test_serverless/**/services/ @elastic/appex-qa |
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 also realised we are not pinged for changes in api_integration/services
.github/CODEOWNERS
Outdated
x-pack/test_serverless @elastic/appex-qa | ||
test @elastic/appex-qa | ||
x-pack/test @elastic/appex-qa | ||
x-pack/performance @elastic/appex-qa |
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 think CI will re-add these because you are the owners of those "packages"
Maybe adding an override negating it?
Or setting those packages' owners as an empty array?
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.
Thanks for explanation, I will try to set owners to empty array and see if that works.
If not working, I will investigate how to override negating it (did you try it before?)
@@ -1263,7 +1259,6 @@ x-pack/test/observability_ai_assistant_functional @elastic/obs-ai-assistant | |||
/test/functional/services/remote @elastic/appex-qa | |||
/test/visual_regression @elastic/appex-qa | |||
/x-pack/test/visual_regression @elastic/appex-qa | |||
/x-pack/performance @elastic/appex-qa |
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.
should be added by CI to the top of the list as I left "package" owner
/ci |
💚 Build Succeeded
Metrics [docs]
To update your PR or re-run it, just comment with: |
x-pack/test_serverless | ||
test | ||
x-pack/test |
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.
automatically added by CI, I guess it is fine as long no one is assigned?
@afharo wdyt?
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.
LGTM
Summary
I noticed
elastic/appex-qa
is pinged for quite many PRs.with #188606 some test folders became packages with
"owner": "@elastic/appex-qa",
, that autmatically updated CODEOWNERS file with appex-qa listed for basically every test path.https://github.com/elastic/kibana/pull/188606/files#diff-3d36a1bf06148bc6ba1ce2ed3d19de32ea708d955fed212c0d27c536f0bd4da7R878-R881
This PR removes
owner
for the following test "packages"and CODEOWNERS file keeps these paths without specific owner.