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

Remove unused legacy import code #111071

Merged
merged 25 commits into from
Sep 7, 2021
Merged

Remove unused legacy import code #111071

merged 25 commits into from
Sep 7, 2021

Conversation

TinaHeiligers
Copy link
Contributor

@TinaHeiligers TinaHeiligers commented Sep 3, 2021 •
edited
Loading

Summary

Follow up PR to initial work for removing support for legacy import.
Removes unused code that remained after support for legacy imports was removed from the top most UI layer of saved objects management.
Handles Phase 2 of #103921

Note to reviews: This worked started before #110738 was merged but has been updated to include the latest on master.

Checklist

Risk Matrix

Risk Probability Severity Mitigation/Notes
Code using legacy format import methods will throw errors Low Low PR's will fail if there are type discrepancies due to methods not being available anymore

For maintainers

TinaHeiligers and others added 18 commits August 31, 2021 17:09
@TinaHeiligers
Removes support for legacy exports
95316f7
@TinaHeiligers
Removes code that hooks into legacy imports
c98a48f
@TinaHeiligers
Updates flyout component unit tests
48076d7
@TinaHeiligers
Removes commented out code from import_mode_control
b6f34d8
@TinaHeiligers
Fix types, fix i18n
b14dd50
@TinaHeiligers
removes another unused const
a284dd6
@TinaHeiligers
Updates test but should remove it because it tests legacy imports
8f5261a
@TinaHeiligers
deletes dashboard bwc_import test and fixture, deletes legacy timezon…
b7e7a18 
…etest fixture
@TinaHeiligers
Deletes timezone test as that used to test against legacy imports
b2d990b
@TinaHeiligers
Updates mgmt import objects fixture to normal format
ffa9bfc
@kibanamachine
Merge branch 'master' into so-management/remove_legacy_import_support
1f61d9e
@TinaHeiligers
Restores dashboard functional tests and skips them for the presentati…
c0299e6 
…on team to address
@TinaHeiligers
Deletes unused json exports
912c588
@TinaHeiligers
Merge branch 'master' of github.com:elastic/kibana into so-management…
7fac104 
…/remove_legacy_import_support
@TinaHeiligers
Removes duplicate code, moves component contents inline
317a448
@TinaHeiligers
Adds comments to methods/types no longer used after legacy import sup…
bb515a6 
…port removed
@TinaHeiligers
Removes unused methods in lib, updates flyout test
4071c98
@TinaHeiligers
Completes first pass at removing unused code from legacy format saved…
7f53830 
… objects import support
@TinaHeiligers TinaHeiligers added release_note:skip Skip the PR/issue when compiling release notes v8.0.0 labels Sep 3, 2021
@TinaHeiligers TinaHeiligers marked this pull request as ready for review September 3, 2021 18:18
@TinaHeiligers TinaHeiligers requested a review from a team as a code owner September 3, 2021 18:18
@TinaHeiligers TinaHeiligers added Feature:Saved Objects techdebt technical debt Improvement of the software architecture and operational architecture and removed techdebt labels Sep 3, 2021
@TinaHeiligers TinaHeiligers marked this pull request as draft September 3, 2021 18:20
@TinaHeiligers TinaHeiligers marked this pull request as ready for review September 3, 2021 18:23
@TinaHeiligers
Copy link
Contributor Author

@elasticmachine merge upstream

pgayvallet
pgayvallet approved these changes Sep 6, 2021
View reviewed changes
Copy link
Contributor

@pgayvallet pgayvallet left a comment

Choose a reason for hiding this comment

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

I couldn't find anything you may have missed during the cleanup, I think you got everything.

src/plugins/saved_objects_management/public/lib/index.ts
Comment on lines -19 to -22
export {
resolveIndexPatternConflicts,
resolveSavedObjects,
resolveSavedSearches,
Copy link
Contributor

Choose a reason for hiding this comment

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

Goodbye resolveSavedObjects, you won't be missed.

...ugins/saved_objects_management/public/management_section/objects_table/components/flyout.tsx Outdated
Comment on lines 71 to 68
conflictedSearchDocs?: any[];
conflictedSearchDocs?: any[]; // potentially unused
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is, I don't see any usages outside of resolve_saved_objects. Any issue when you tried to remove it?

@TinaHeiligers TinaHeiligers enabled auto-merge (squash) September 6, 2021 18:24
@TinaHeiligers
Copy link
Contributor Author

@elasticmachine merge upstream

@TinaHeiligers
Copy link
Contributor Author

TinaHeiligers commented Sep 7, 2021 •
edited
Loading

@rudolf it looks like we've got a flaky integration test with migration batching. Could you please take a look when you get a chance?

@pgayvallet
Copy link
Contributor

retest

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
savedObjectsManagement 83 80 -3

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
savedObjectsManagement 93 91 -2

Async chunks

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

id before after diff
savedObjectsManagement 131.1KB 131.0KB -139.0B

Page load bundle

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

id before after diff
savedObjectsManagement 37.5KB 30.2KB -7.3KB
Unknown metric groups

API count

id before after diff
savedObjectsManagement 106 104 -2

References to deprecated APIs

id before after diff
savedObjectsManagement 74 29 -45

History

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

@TinaHeiligers TinaHeiligers merged commit b1a81bb into elastic:master Sep 7, 2021
@TinaHeiligers TinaHeiligers deleted the tech-debt/remove_unused_lib_legacy_code branch September 7, 2021 14:01
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 111071 or prevent reminders by adding the backport:skip label.

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Sep 9, 2021
@pgayvallet pgayvallet added backport:skip This commit does not require backporting and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Sep 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pgayvallet pgayvallet pgayvallet approved these changes

Assignees
No one assigned
Labels
backport:skip This commit does not require backporting Feature:Saved Objects release_note:skip Skip the PR/issue when compiling release notes technical debt Improvement of the software architecture and operational architecture v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@TinaHeiligers @pgayvallet @kibanamachine