Skip to content

[core] remove root index.ts file#137001

Merged
spalger merged 3 commits intoelastic:mainfrom
spalger:fix/core/no-root-plugin-imports
Jul 22, 2022
Merged

[core] remove root index.ts file#137001
spalger merged 3 commits intoelastic:mainfrom
spalger:fix/core/no-root-plugin-imports

Conversation

@spalger
Copy link
Copy Markdown
Contributor

@spalger spalger commented Jul 22, 2022

It's unclear what the purpose of this file is, but I can't find anyone using it and this file imports server and public code at the same time so it needs to be fixed for #136911. Figured I would just delete it and see what y'all think.

@spalger spalger added Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// release_note:skip Skip the PR/issue when compiling release notes backport:prev-minor labels Jul 22, 2022
@spalger spalger requested a review from a team as a code owner July 22, 2022 19:58
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/kibana-core (Team:Core)

@TinaHeiligers
Copy link
Copy Markdown
Contributor

@spalger the root index file allows import {CoreStart} from 'kibana/public', ref: #32746.

@spalger
Copy link
Copy Markdown
Contributor Author

spalger commented Jul 22, 2022

@TinaHeiligers kibana/public resolves to the src/core/public directory, so this file isn't important for that. Additionally, in #130483 we enforced uniform import statements and these import statements were all rewritten to @kbn/core/public then and going forward.

@TinaHeiligers
Copy link
Copy Markdown
Contributor

There are still a few places where import XYZ from 'kibana/server|public' is used. We'd need to update those, especially in the docs and code snippets!

@spalger
Copy link
Copy Markdown
Contributor Author

spalger commented Jul 22, 2022

I can submit a PR to replace those, but like I said, these don't interact with src/core/index.ts, they import src/core/public/index.ts and src/core/server/index.ts

Copy link
Copy Markdown
Contributor

@TinaHeiligers TinaHeiligers left a comment

Choose a reason for hiding this comment

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

The code snippets, example plugins and docs need to reflect this change.
Update: I'm changing this to a comment rather than an explicit change request.

@TinaHeiligers TinaHeiligers self-requested a review July 22, 2022 20:54
Copy link
Copy Markdown
Contributor

@TinaHeiligers TinaHeiligers left a comment

Choose a reason for hiding this comment

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

We need to make absolutely sure the docs and code examples/snippets are updated at the same (or very shortly after) this gets merged.
Will I get this right sometime today?

@TinaHeiligers TinaHeiligers self-requested a review July 22, 2022 20:56
@spalger spalger requested a review from a team as a code owner July 22, 2022 20:56
Copy link
Copy Markdown
Contributor

@TinaHeiligers TinaHeiligers left a comment

Choose a reason for hiding this comment

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

Another go at a plain comment. See previous comments..

@spalger
Copy link
Copy Markdown
Contributor Author

spalger commented Jul 22, 2022

Alright @TinaHeiligers I updated the docs and removed support for kibana/* imports

@TinaHeiligers TinaHeiligers requested a review from a team July 22, 2022 20:57
Copy link
Copy Markdown
Contributor

@TinaHeiligers TinaHeiligers left a comment

Choose a reason for hiding this comment

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

LGTM!

@kibana-ci
Copy link
Copy Markdown

💚 Build Succeeded

Metrics [docs]

✅ unchanged

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

@spalger spalger merged commit 03f1a21 into elastic:main Jul 22, 2022
@spalger spalger deleted the fix/core/no-root-plugin-imports branch July 22, 2022 22:04
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jul 22, 2022
* [core] remove root index.ts file

* remove support for `kibana/*` imports, replace instances in docs

(cherry picked from commit 03f1a21)
@kibanamachine
Copy link
Copy Markdown
Contributor

💚 All backports created successfully

Status Branch Result
8.3

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Jul 22, 2022
* [core] remove root index.ts file

* remove support for `kibana/*` imports, replace instances in docs

(cherry picked from commit 03f1a21)

Co-authored-by: Spencer <spencer@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release_note:skip Skip the PR/issue when compiling release notes Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// v8.3.3 v8.4.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants