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.

Sign up for GitHub

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

Jump to bottom

[Enterprise Search] More granular CODEOWNERS notifications #84713

Merged
merged 1 commit into from
Dec 1, 2020
Merged

[Enterprise Search] More granular CODEOWNERS notifications #84713

merged 1 commit into from
Dec 1, 2020

Conversation

cee-chen
Copy link
Contributor

@cee-chen cee-chen commented Dec 1, 2020 •
edited
Loading

Summary

This is a bit of a doozy, but until Kibana adds the option of registering multiple plugins per folder (which I believe is coming soon), I think this is the best we got 🤔

  • Upside: avoids requiring an approval from two separate devs / both App Search and Workplace Search teams when touching shared files (this was the problem that we disabled in [Enterprise Search] Update CODEOWNERS and README #78809)
  • Downside: if someone outside the team adds a brand new folder to our plugin that we aren't watching in our CODEOWNERS, we don't get notified about it (this seems very unlikely though so I'm not too worried about it)

Let me know what y'all think!

@cee-chen cee-chen added Feature:Plugins v8.0.0 release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting v7.11.0 labels Dec 1, 2020
scottybollinger
scottybollinger approved these changes Dec 1, 2020
View reviewed changes
Copy link
Contributor

@scottybollinger scottybollinger left a comment

Choose a reason for hiding this comment

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

LGTM!

@scottybollinger
Copy link
Contributor

@constancecchen just curious, why to some have * and some don't?

@cee-chen
Copy link
Contributor Author

cee-chen commented Dec 1, 2020 •
edited
Loading

The * specifies files within that directory but not within subdirectories, e.g. we want to track public/plugin.ts & public/index.ts for @elastic/enterprise-search-frontend but not all public/ sub-directories because those need to be granularly added.

GH docs link: https://docs.github.com/en/free-pro-team@latest/github/creating-cloning-and-archiving-repositories/about-code-owners#codeowners-syntax

@cee-chen
Copy link
Contributor Author

cee-chen commented Dec 1, 2020

Here goes nothing! If we have any approval/codeowners issues we will continue to tweak.

@cee-chen cee-chen merged commit 00a5b60 into elastic:master Dec 1, 2020
@cee-chen cee-chen deleted the ent-search-frontend-codeowners branch December 1, 2020 22:59
gmmorris added a commit to gmmorris/kibana that referenced this pull request Dec 2, 2020
@gmmorris
Merge branch 'master' into alerts/custom-recovery-action-group
b92d4b8 
* master:
  [Lens] Show color in flyout instead of auto (elastic#84532)
  [Lens] Use index pattern through service instead of reading saved object (elastic#84432)
  Make it possible to use Kibana anonymous authentication provider with ES anonymous access. (elastic#84074)
  TelemetryCollectionManager: Use X-Pack strategy as an OSS overwrite (elastic#84477)
  migrate away from rest_total_hits_as_int (elastic#84508)
  [Input Control] Custom renderer (elastic#84423)
  Attempt to more granularly separate App Search vs Workplace Search vs shared GitHub notifications (elastic#84713)
  [Security Solutino][Case] Case connector alert UI (elastic#82405)
  [Maps] Support runtime fields in tooltips (elastic#84377)
  [CCR] Fix row actions in follower index and auto-follow pattern tables (elastic#84433)
  [Enterprise Search] Migrate shared Indexing Status component (elastic#84571)
  [maps] remove fields from index-pattern test artifacts (elastic#84379)
  Add routes for use in Sources Schema (elastic#84579)
  Changes UI links for drilldowns (elastic#83971)
  endpoint telemetry cloned endpoint tests (elastic#81498)
  [Fleet] Handler api key creation errors when Fleet Admin is invalid (elastic#84576)
gmmorris added a commit to gmmorris/kibana that referenced this pull request Dec 2, 2020
@gmmorris
Merge branch 'master' into alerts/force-schedule
261977c 
* master: (72 commits)
  Make alert status fetching more resilient (elastic#84676)
  [APM] Refactor hooks and context (elastic#84615)
  Added word break styles to the texts in the item details card. (elastic#84654)
  [Search] Disable "send to background" when auto-refresh is enabled (elastic#84106)
  Add readme for new palette service (elastic#84512)
  Make all providers to preserve original URL when session expires. (elastic#84229)
  [Lens] Show color in flyout instead of auto (elastic#84532)
  [Lens] Use index pattern through service instead of reading saved object (elastic#84432)
  Make it possible to use Kibana anonymous authentication provider with ES anonymous access. (elastic#84074)
  TelemetryCollectionManager: Use X-Pack strategy as an OSS overwrite (elastic#84477)
  migrate away from rest_total_hits_as_int (elastic#84508)
  [Input Control] Custom renderer (elastic#84423)
  Attempt to more granularly separate App Search vs Workplace Search vs shared GitHub notifications (elastic#84713)
  [Security Solutino][Case] Case connector alert UI (elastic#82405)
  [Maps] Support runtime fields in tooltips (elastic#84377)
  [CCR] Fix row actions in follower index and auto-follow pattern tables (elastic#84433)
  [Enterprise Search] Migrate shared Indexing Status component (elastic#84571)
  [maps] remove fields from index-pattern test artifacts (elastic#84379)
  Add routes for use in Sources Schema (elastic#84579)
  Changes UI links for drilldowns (elastic#83971)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scottybollinger scottybollinger scottybollinger approved these changes

Assignees
No one assigned
Labels
backport:skip This commit does not require backporting Feature:Plugins release_note:skip Skip the PR/issue when compiling release notes v7.11.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@cee-chen @scottybollinger