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

Cleanup of the kibanaLegacy unused functions and remove unecessary dependencies from Analytics plugins #113358

Merged
merged 5 commits into from
Sep 30, 2021

Conversation

stratoula
Copy link
Contributor

@stratoula stratoula commented Sep 29, 2021

Summary

Part of #76905
By removing angular from our plugins, we also no need to depend on the kibanaLegacy plugin. This PR, removes the dependency from the Analytics teams plugins as it was not used anymore.

Also it removes the unused functions from the kibana_legacy plugin. A second PR will follow after angular is removed from the monitoring plugin which will delete all the remaining angular from the plugin.

Our ultimate goal is to remove the kibana_legacy plugin.

@stratoula stratoula changed the title [Discover][Table] Remove unused dependencies of the kibanaLegacy plugin Removes unused dependencies of the kibanaLegacy from Analytics plugins Sep 29, 2021
@stratoula stratoula changed the title Removes unused dependencies of the kibanaLegacy from Analytics plugins Cleanup of the kibanaLegacy unused functions and remove unecessary dependencies from Analytics plugins Sep 29, 2021
@stratoula
Copy link
Contributor Author

stratoula commented Sep 29, 2021

I haven't removed the dependency from the discover plugin as this function src/plugins/kibana_legacy/public/utils/inject_header_style.ts is needed for the truncate:maxHeight UI setting to work correctly. As the kibanaLegacy plugin is not disableable maybe it is safe to remove the dependency from the discover plugin but I am not 100% sure. @elastic/kibana-data-discovery wdyt?

Update: We discussed it offline. We are going to remove the dependency when the above functionality will be moved in the discover plugin.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
kibanaLegacy 58 35 -23

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
discoverEnhanced 37 35 -2
kibanaLegacy 64 45 -19
total -21

Any counts in public APIs

Total count of every any typed public API. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats any for more detailed information.

id before after diff
kibanaLegacy 3 1 -2

Async chunks

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

id before after diff
kibanaLegacy 122.7KB 118.2KB -4.5KB

Page load bundle

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

id before after diff
kibanaLegacy 45.7KB 22.5KB -23.2KB
Unknown metric groups

API count

id before after diff
discoverEnhanced 39 37 -2
kibanaLegacy 68 48 -20
total -22

async chunk count

id before after diff
kibanaLegacy 2 1 -1

History

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

@stratoula stratoula added Feature:Legacy Removal Issues related to removing legacy Kibana Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.16.0 v8.0.0 release_note:skip Skip the PR/issue when compiling release notes labels Sep 29, 2021
@stratoula stratoula marked this pull request as ready for review September 29, 2021 12:40
@stratoula stratoula requested review from a team as code owners September 29, 2021 12:40
Copy link
Contributor

@mbondyra mbondyra left a comment

Choose a reason for hiding this comment

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

Lovely PR 👏🏼 I think CI and functional tests give enough confidence to not even test it in this case but I ran it locally and opened all the involved parties (dashboard etc). Works with no problems!

Copy link
Contributor

@ThomThomson ThomThomson left a comment

Choose a reason for hiding this comment

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

This is amazing! Looked over the dashboard changes, and it's incredible that I didn't notice we weren't using Kibana_legacy anymore. Thank you for removing it, and nothing is better than seeing +2 -1660 on a PR.

L G T M

Copy link
Member

@kertal kertal left a comment

Choose a reason for hiding this comment

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

LGTM, reviewed data discovery owned code. didn't test, but the usage of 'kibanaLegacy' in 'discover_enhanced' was always redundant, thx for the cleanup

@stratoula stratoula added the auto-backport Deprecated - use backport:version if exact versions are needed label Sep 30, 2021
@stratoula stratoula merged commit a9d80ac into elastic:master Sep 30, 2021
@kibanamachine
Copy link
Contributor

💔 Backport failed

Status Branch Result
7.x Commit could not be cherrypicked due to conflicts

To backport manually run:
node scripts/backport --pr 113358

stratoula added a commit to stratoula/kibana that referenced this pull request Sep 30, 2021
…pendencies from Analytics plugins (elastic#113358)

* [Discover][Table] Remove unused dependencies of the kibanaLegacy plugin

* More removals of kibanaLegacy plugin dependencies

* Revert discover changes

* Remove the unused functions from the kibana_legacy plugin

* Removes unused translations
# Conflicts:
#	src/plugins/kibana_legacy/public/plugin.ts
stratoula added a commit that referenced this pull request Sep 30, 2021
…ary dependencies from Analytics plugins (#113358) (#113503)

* Cleanup of the kibanaLegacy unused functions and remove unecessary dependencies from Analytics plugins (#113358)

* [Discover][Table] Remove unused dependencies of the kibanaLegacy plugin

* More removals of kibanaLegacy plugin dependencies

* Revert discover changes

* Remove the unused functions from the kibana_legacy plugin

* Removes unused translations
# Conflicts:
#	src/plugins/kibana_legacy/public/plugin.ts

* Revert usage that cant be removed in 7.x
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed Feature:Legacy Removal Issues related to removing legacy Kibana release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.16.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants