Skip to content

Conversation

@kindsun
Copy link
Contributor

@kindsun kindsun commented Jan 4, 2021

Sets the Maps team as code owners of the two geo alerts. Didn't update code coverage since it's covered one level higher for the stack_alerts plugin by the alerting team.

@kindsun kindsun added Team:Geo Former Team Label for Geo Team. Now use Team:Presentation Feature:Alerting v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Platform ResponseOps team (formerly the Cases and Alerting teams) t// v7.12.0 labels Jan 4, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-gis (Team:Geo)

@elasticmachine
Copy link
Contributor

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

Copy link
Contributor

@YulNaumenko YulNaumenko left a comment

Choose a reason for hiding this comment

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

LGTM

@thomasneirynck
Copy link
Contributor

I'd add #CC# tags too, so we get code coverage stats on them

@kindsun
Copy link
Contributor Author

kindsun commented Jan 4, 2021

I'd add #CC# tags too, so we get code coverage stats on them

@thomasneirynck Happy to, just want to check one detail. I actually left #CC# out since it's already done one level higher for the entire plugin here, albeit by the alerting team:

#CC# /x-pack/plugins/stack_alerts/ @elastic/kibana-alerting-services
Should we overlap coverage?

@thomasneirynck
Copy link
Contributor

wrt #87188 (comment)

good point. I don't know. Basically, we'd want these directories to to be tracked under the gis-team as well, so code-coverage stats for the gis-team capture include these dirs. cc @LeeDr do you know if this would cause problems for stats-reporting?

@LeeDr
Copy link

LeeDr commented Jan 5, 2021

I think the comments on the CODEOWNERS file could be a little more clear. They currently say;

# GitHub CODEOWNERS definition
# Identify which groups will be pinged by changes to different parts of the codebase.
# For more info, see https://help.github.com/articles/about-codeowners/

# The #CC# prefix delineates Code Coverage,
# used for the 'team' designator within Kibana Stats

I'd like @wayneseymour to confirm but I think this is true;

  • the lines which don't start with #CC# are used for code coverage AND trigger code reviews
  • the lines which do start with #CC# are only used for code coverage

Multiple teams can be listed both with and without the #CC# and all are combined for code coverage (the paths will be included when filtering by any of those teams).

Or do I have this wrong and the #CC# overrides the non-#CC# lines so that we could assign a single code coverage team even if multiple teams are triggered for code reviews?

@wayneseymour
Copy link
Contributor

  • the lines which don't start with #CC# are used for code coverage AND trigger code reviews
  • the lines which do start with #CC# are only used for code coverage

Correct!

@kindsun
Copy link
Contributor Author

kindsun commented Jan 5, 2021

Sounds like we're in good shape then. Thanks all!

@kindsun kindsun merged commit 437a201 into elastic:master Jan 5, 2021
@kindsun kindsun deleted the add-geo-alerts-to-cc-for-maps-team branch January 5, 2021 21:55
@wayneseymour
Copy link
Contributor

@kindsun kindsun added backport:skip This PR does not require backporting and removed v7.12.0 labels Jan 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting Feature:Alerting release_note:skip Skip the PR/issue when compiling release notes Team:Geo Former Team Label for Geo Team. Now use Team:Presentation Team:ResponseOps Platform ResponseOps team (formerly the Cases and Alerting teams) t// v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants