-
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
[UnifiedDocViewer] Move Discover doc viewer into plugin/package #162847
Conversation
…/unified-doc-viewer
… into pkg/unified-doc-viewer
Pinging @elastic/kibana-data-discovery (Team:DataDiscovery) |
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.
The latest changes look good! I like that we're abstracting away the services and registry, and this API feels more in line with some of the other AnalystXP components. I also tested locally again and things still seem to be working as expected. I just have a couple of minor comments on the code.
src/plugins/discover/public/components/doc_table/doc_table_wrapper.tsx
Outdated
Show resolved
Hide resolved
Sorry @walterra, I didn't mean to re-request your review, clicked on accident! |
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 the last bit of cleanup! I did a final round of testing including Discover, Surrounding Docs, and Dashboard with both the new data grid and the legacy one and confirmed everything seems to be working as expected. Thanks for all the great work on this, LGTM 👍
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.
Looking great 🥳 Thx so much for bringing this over the finishing line! Just left some minor remarks having another look at the code
💔 Build FailedFailed CI Steps
Test Failures
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Public APIs missing exports
Page load bundle
Unknown metric groupsAPI count
async chunk count
ESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: cc @lukasolson |
💔 All backports failed
Manual backportTo create the backport manually run:
Questions ?Please refer to the Backport tool documentation |
Summary
Replaces #154012.
Moves the Discover doc viewer component into a new plugin/package,
@kbn/unified-doc-viewer
and@kbn/unified-doc-viewer-plugin
.Checklist
Risk Matrix
Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.
When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:
For maintainers