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

Remove bfetch plugin #204285

Merged
merged 17 commits into from
Dec 19, 2024
Merged

Remove bfetch plugin #204285

merged 17 commits into from
Dec 19, 2024

Conversation

lukasolson
Copy link
Member

@lukasolson lukasolson commented Dec 13, 2024

Summary

Part of #186139.

Relies on #204284.

Second step of breaking up #199066 into smaller pieces.

Removes the bfetch and bfetch-error plugins.

@lukasolson lukasolson added Feature:Search Querying infrastructure in Kibana release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. labels Dec 13, 2024
@lukasolson lukasolson self-assigned this Dec 13, 2024
@lukasolson lukasolson marked this pull request as ready for review December 18, 2024 16:15
@lukasolson lukasolson requested review from a team as code owners December 18, 2024 16:15
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

@botelastic botelastic bot added Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) Team:obs-ux-management Observability Management User Experience Team labels Dec 18, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-management-team (Team:obs-ux-management)

Copy link
Member

@pheyos pheyos left a comment

Choose a reason for hiding this comment

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

x-pack/test/tsconfig.json changes LGTM

Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

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

Would it be possible to refactor x-pack/test/api_integration/apis/maps/bsearch.ts to use search API? These tests have value ensuring responses required by maps do not change shape.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
bfetch 27 - -27

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
@kbn/bfetch-error 1 - -1
bfetch 59 - -59
total -60

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
bfetch 1 - -1

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
bfetch 2 - -2

Page load bundle

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

id before after diff
bfetch 6.8KB - -6.8KB
kbnUiSharedDeps-srcJs 3.5MB 3.5MB -390.0B
total -7.2KB
Unknown metric groups

API count

id before after diff
@kbn/bfetch-error 4 - -4
bfetch 60 - -60
total -64

ESLint disabled in files

id before after diff
bfetch 1 - -1

ESLint disabled line counts

id before after diff
bfetch 4 - -4

Total ESLint disabled count

id before after diff
bfetch 5 - -5

History

cc @lukasolson

@lukasolson
Copy link
Member Author

Would it be possible to refactor x-pack/test/api_integration/apis/maps/bsearch.ts to use search API? These tests have value ensuring responses required by maps do not change shape.

👍 8ecf1af

@lukasolson lukasolson requested a review from nreese December 18, 2024 19:16
Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

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

kibana-presentation changes LGTM
code review only

@lukasolson lukasolson mentioned this pull request Dec 18, 2024
6 tasks
Copy link
Member

@afharo afharo left a comment

Choose a reason for hiding this comment

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

kibana.jsonc changes LGTM

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.

codeowner changes lgtm

@mattkime
Copy link
Contributor

Please, out of respect for the proud legacy of the bfetch code, play this while merging - https://www.youtube.com/watch?v=2d37ij4EWGg

Copy link
Member

@markov00 markov00 left a comment

Choose a reason for hiding this comment

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

Visualization code changes look good to me!

Copy link
Contributor

@eokoneyo eokoneyo left a comment

Choose a reason for hiding this comment

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

Changes; deletions of the bfetch plugin, LGTM

Copy link
Contributor

@jughosta jughosta left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Member

@jasonrhodes jasonrhodes left a comment

Choose a reason for hiding this comment

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

Re: obs-ux-management changes and removing stale refs: I grepped our plugins for bfetch references and saw none and Kibana loads with no errors. LGTM!

@lukasolson lukasolson merged commit 9ad31d0 into elastic:main Dec 19, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) Feature:Search Querying infrastructure in Kibana release_note:skip Skip the PR/issue when compiling release notes Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. Team:obs-ux-management Observability Management User Experience Team v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.