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

Management app locator #101795

Merged
merged 12 commits into from
Jun 14, 2021
Merged

Management app locator #101795

merged 12 commits into from
Jun 14, 2021

Conversation

streamich
Copy link
Contributor

@streamich streamich commented Jun 9, 2021 •
edited
Loading

Summary

Adds locator to management plugin, needed for #98107 for ILM locator.

Checklist

Delete any items that are not applicable to this PR.

@streamich streamich requested a review from a team June 9, 2021 16:19
@streamich streamich requested a review from a team as a code owner June 9, 2021 16:19
@streamich streamich requested a review from mattkime June 9, 2021 16:19
@streamich streamich added release_note:skip Skip the PR/issue when compiling release notes review Team:AppServices v7.14.0 v8.0.0 labels Jun 9, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-services (Team:AppServices)

@streamich streamich requested a review from ppisljar June 10, 2021 13:15
ppisljar
ppisljar approved these changes Jun 10, 2021
View reviewed changes
Copy link
Member

@ppisljar ppisljar left a comment

Choose a reason for hiding this comment

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

code LGTM

@streamich streamich requested a review from a team as a code owner June 10, 2021 14:14
legrego
legrego approved these changes Jun 10, 2021
View reviewed changes
Copy link
Member

@legrego legrego left a comment

Choose a reason for hiding this comment

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

security changes LGTM

mattkime
mattkime approved these changes Jun 10, 2021
View reviewed changes
Copy link
Contributor

@mattkime mattkime left a comment

Choose a reason for hiding this comment

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

👍

@streamich
Copy link
Contributor Author

@elasticmachine merge upstream

timroes
timroes approved these changes Jun 14, 2021
View reviewed changes
Copy link
Contributor

@timroes timroes left a comment

Choose a reason for hiding this comment

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

Left some minor comment about proper type imports, but other than that code LGTM. Haven't checked out PR and tested, only looked at CODEOWNER files from app.

src/plugins/management/public/plugin.ts Outdated Show resolved Hide resolved
src/plugins/management/public/types.ts Outdated Show resolved Hide resolved
src/plugins/management/public/types.ts Outdated Show resolved Hide resolved
src/plugins/management/server/plugin.ts Outdated Show resolved Hide resolved
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
management 34 35 +1

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
management 38 39 +1
share 61 68 +7
total +8

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
management 4 5 +1
share 4 6 +2
total +3

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
management 15.6KB 16.2KB +678.0B
share 71.3KB 71.8KB +599.0B
total +1.2KB
Unknown metric groups

API count

id before after diff
management 38 39 +1
share 67 85 +18
total +19

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@streamich streamich merged commit fa855b3 into elastic:master Jun 14, 2021
@streamich streamich mentioned this pull request Jun 14, 2021
Merged
streamich added a commit that referenced this pull request Jun 14, 2021
@streamich @kibanamachine @timroes
Management app locator (#101795) (#102049)
b1c0975 
* feat: 🎸 create management app locator

* refactor: 💡 simplify management locator

* feat: 🎸 export management app locator from plugin contract

* feat: 🎸 improve share plugin exports

* test: 💍 fix test mock

* test: 💍 adjust test mocks

* Update src/plugins/management/public/plugin.ts

Co-authored-by: Tim Roes <[email protected]>

* Update src/plugins/management/public/types.ts

Co-authored-by: Tim Roes <[email protected]>

* Update src/plugins/management/public/types.ts

Co-authored-by: Tim Roes <[email protected]>

* Update src/plugins/management/server/plugin.ts

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Tim Roes <[email protected]>

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Tim Roes <[email protected]>
cuff-links pushed a commit to cuff-links/kibana that referenced this pull request Jun 15, 2021
@streamich @kibanamachine @timroes
Management app locator (elastic#101795)
03f97f7 
* feat: 🎸 create management app locator

* refactor: 💡 simplify management locator

* feat: 🎸 export management app locator from plugin contract

* feat: 🎸 improve share plugin exports

* test: 💍 fix test mock

* test: 💍 adjust test mocks

* Update src/plugins/management/public/plugin.ts

Co-authored-by: Tim Roes <[email protected]>

* Update src/plugins/management/public/types.ts

Co-authored-by: Tim Roes <[email protected]>

* Update src/plugins/management/public/types.ts

Co-authored-by: Tim Roes <[email protected]>

* Update src/plugins/management/server/plugin.ts

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Tim Roes <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattkime mattkime mattkime approved these changes

@timroes timroes timroes approved these changes

@legrego legrego legrego approved these changes

@ppisljar ppisljar ppisljar approved these changes

Assignees
No one assigned
Labels
release_note:skip Skip the PR/issue when compiling release notes review v7.14.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@streamich @elasticmachine @kibanamachine @mattkime @timroes @legrego @ppisljar