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.

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

Use doc link services in index management #89957

Merged
merged 11 commits into from
Apr 13, 2021

Conversation

lcawl
Copy link
Contributor

@lcawl lcawl commented Feb 2, 2021

Summary

Related to #88107

This PR removes hard-coded links from x-pack/plugins/index_management/public/application/components/component_templates/lib/documentation.ts and x-pack/plugins/index_management/public/application/services/documentation.ts, replacing them with keywords from the documentation link service.

In the long run, these documentation.ts files should likely be removed and the doc link service called directly instead.

@lcawl
Copy link
Contributor Author

lcawl commented Feb 6, 2021

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

merge conflict between base and head

@lcawl lcawl force-pushed the doc-links-index-management branch from a0c3e2a to 5aca330 Compare March 26, 2021 21:49
@lcawl
Copy link
Contributor Author

lcawl commented Mar 29, 2021

@elasticmachine merge upstream

@lcawl lcawl marked this pull request as ready for review March 29, 2021 23:34
@lcawl lcawl requested review from a team as code owners March 29, 2021 23:34
@lcawl lcawl added Feature:Index Management Index and index templates UI v7.13.0 v8.0.0 release_note:skip Skip the PR/issue when compiling release notes labels Mar 29, 2021
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.

changes lgtm

@mattkime
Copy link
Contributor

@elasticmachine merge upstream

Copy link
Contributor

@alisonelizabeth alisonelizabeth left a comment

Choose a reason for hiding this comment

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

Thanks for working on this @lcawl! Index Management has a lot of doc links to cover 😄 .

In the long run, these documentation.ts files should likely be removed and the doc link service called directly instead.

I agree. I started looking into addressing this myself on your branch, but it will require touching a number of files and I think might be more appropriate to address as a separate PR. I can open up an issue to track this work.

I also noticed there are still a handful of hard-coded links generated using esDocsBath (it looks confined to the step_ components - step_aliases, step_settings, step_mappings). Should these be added to the core doc links service as well?

@lcawl
Copy link
Contributor Author

lcawl commented Apr 12, 2021

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

merge conflict between base and head

@lcawl
Copy link
Contributor Author

lcawl commented Apr 12, 2021

I also noticed there are still a handful of hard-coded links generated using esDocsBase (it looks confined to the step_ components - step_aliases, step_settings, step_mappings). Should these be added to the core doc links service as well?

Yes, good catch. I added keywords for those hard-coded links in the core doc link service, but haven't updated the TSX files to use them. It seems to me that would make sense to do after this app is using the core doc link service in general. If you disagree, let me know and I can add that to this PR.

Copy link
Contributor

@alisonelizabeth alisonelizabeth left a comment

Choose a reason for hiding this comment

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

Latest LGTM. Thanks again for working on this @lcawl! I left one question about the doc link for the boost parameter. Would you mind taking a look before merging?

I also opened #96910 as a follow up to remove the documentation service.

@lcawl
Copy link
Contributor Author

lcawl commented Apr 13, 2021

@elasticmachine update branch

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
dataEnhanced 159.1KB 159.0KB -171.0B
indexManagement 1.3MB 1.3MB +3.8KB
total +3.6KB

Page load bundle

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

id before after diff
core 377.0KB 379.7KB +2.7KB

History

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

@lcawl lcawl merged commit 27cd514 into elastic:master Apr 13, 2021
@lcawl lcawl deleted the doc-links-index-management branch April 13, 2021 17:21
lcawl added a commit to lcawl/kibana that referenced this pull request Apr 13, 2021
Co-authored-by: Alison Goryachev <alisonmllr20@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Index Management Index and index templates UI release_note:skip Skip the PR/issue when compiling release notes v7.13.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants