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 kbn-ace, ace and brace dependencies #195703

Merged
merged 7 commits into from
Oct 10, 2024

Conversation

sabarasaba
Copy link
Member

@sabarasaba sabarasaba commented Oct 10, 2024

Closes #194533

Summary

With the effort started with #194533 and #155833 we have now finally migrated all instances of the Ace editor into Monaco and there are no more plugins that depend on it.

This PR removes kbn-package, ace and brace dependency from the kibana project.

@sabarasaba sabarasaba added Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting labels Oct 10, 2024
@sabarasaba sabarasaba self-assigned this Oct 10, 2024
Copy link
Contributor

A documentation preview will be available soon.

Request a new doc build by commenting
  • Rebuild this PR: run docs-build
  • Rebuild this PR and all Elastic docs: run docs-build rebuild

run docs-build is much faster than run docs-build rebuild. A rebuild should only be needed in rare situations.

If your PR continues to fail for an unknown reason, the doc build pipeline may be broken. Elastic employees can check the pipeline status here.

@sabarasaba
Copy link
Member Author

@elasticmachine merge upstream

@sabarasaba sabarasaba marked this pull request as ready for review October 10, 2024 08:58
@sabarasaba sabarasaba requested review from a team as code owners October 10, 2024 08:58
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-management (Team:Kibana Management)

@sabarasaba sabarasaba requested a review from a team as a code owner October 10, 2024 08:58
Copy link
Member

@dmlemeshko dmlemeshko left a comment

Choose a reason for hiding this comment

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

packages/kbn-test changes LGTM

@sabarasaba sabarasaba added backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) v8.16.0 and removed backport:skip This commit does not require backporting labels Oct 10, 2024
Copy link
Member

@azasypkin azasypkin left a comment

Choose a reason for hiding this comment

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

Changes in role_mappings/edit_role_mapping/rule_editor_panel/json_rule_editor LGTM and the editor still works as expected, thanks!

Copy link
Contributor

@adcoelho adcoelho left a comment

Choose a reason for hiding this comment

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

response-ops changes ok 👍 Some tests but they don't seem connected to the failing alerting api integration security and spaces enabled - Group 2 Alerts legacy alerts alerts "after all" hook in "alerts". Maybe merge main?

@sabarasaba
Copy link
Member Author

@elasticmachine merge upstream

Copy link
Contributor

@ElenaStoeva ElenaStoeva 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

@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.

It seems @elastic/kibana-data-discovery was just pinged because of a data view test change, code owner review. LGTM. Congratulations on removing code 🎉 !

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
apm 1843 1844 +1
canvas 1285 1286 +1
cloudFullStory 11 12 +1
crossClusterReplication 134 133 -1
dashboard 665 666 +1
dataVisualizer 726 727 +1
esUiShared 241 220 -21
ingestPipelines 328 329 +1
integrationAssistant 564 565 +1
lens 1462 1463 +1
ml 2033 2034 +1
savedObjectsManagement 114 115 +1
security 527 524 -3
share 92 93 +1
stackAlerts 161 160 -1
visDefaultEditor 205 203 -2
total -17

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/ace 11 - -11

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
@kbn/ace 5 - -5

Async chunks

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

id before after diff
apm 3.4MB 3.4MB +48.0B
canvas 1.1MB 1.1MB +48.0B
cloudFullStory 13.5KB 17.6KB +4.1KB
crossClusterReplication 148.8KB 146.2KB -2.6KB
dashboard 627.8KB 637.2KB +9.4KB
dataVisualizer 613.8KB 613.8KB +48.0B
ingestPipelines 375.4KB 375.4KB +48.0B
integrationAssistant 949.2KB 949.3KB +49.0B
lens 1.5MB 1.5MB +48.0B
ml 4.5MB 4.5MB +48.0B
savedObjectsManagement 84.5KB 84.6KB +49.0B
security 591.1KB 541.9KB -49.2KB
stackAlerts 75.7KB 73.5KB -2.1KB
visDefaultEditor 142.2KB 95.2KB -47.0KB
total -87.0KB

Page load bundle

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

id before after diff
core 455.7KB 449.4KB -6.3KB
esUiShared 158.4KB 106.7KB -51.7KB
kbnUiSharedDeps-npmDll 6.2MB 5.8MB -417.3KB
share 57.1KB 57.2KB +51.0B
total -475.2KB
Unknown metric groups

API count

id before after diff
@kbn/ace 11 - -11

ESLint disabled in files

id before after diff
@kbn/ace 1 - -1

ESLint disabled line counts

id before after diff
@kbn/ace 1 - -1

Total ESLint disabled count

id before after diff
@kbn/ace 2 - -2

History

cc @sabarasaba

@sabarasaba sabarasaba merged commit d86ce77 into elastic:main Oct 10, 2024
38 of 39 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11273823083

@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.x Backport failed because of merge conflicts

You might need to backport the following PRs to 8.x:
- Fix theme switch success toast layout (#195717)
- Removes supertest and superuser from platform security serverless API tests (#194922)
- [[ES

Manual backport

To create the backport manually run:

node scripts/backport --pr 195703

Questions ?

Please refer to the Backport tool documentation

@sabarasaba
Copy link
Member Author

💚 All backports created successfully

Status Branch Result
8.x

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

Questions ?

Please refer to the Backport tool documentation

sabarasaba added a commit to sabarasaba/kibana that referenced this pull request Oct 10, 2024
(cherry picked from commit d86ce77)

# Conflicts:
#	.github/CODEOWNERS
#	api_docs/kbn_ace.mdx
@sabarasaba
Copy link
Member Author

💚 All backports created successfully

Status Branch Result
8.x

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

Questions ?

Please refer to the Backport tool documentation

sabarasaba added a commit that referenced this pull request Oct 11, 2024
# Backport

This will backport the following commits from `main` to `8.x`:
- [Remove kbn-ace, ace and brace dependencies
(#195703)](#195703)

<!--- Backport version: 8.9.8 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Ignacio
Rivas","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-10T12:16:42Z","message":"Remove
kbn-ace, ace and brace dependencies
(#195703)","sha":"d86ce77217a26747b39ddf240e5703efba1a0cb0","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Kibana
Management","release_note:skip","v9.0.0","backport:prev-minor","v8.16.0"],"number":195703,"url":"https://github.com/elastic/kibana/pull/195703","mergeCommit":{"message":"Remove
kbn-ace, ace and brace dependencies
(#195703)","sha":"d86ce77217a26747b39ddf240e5703efba1a0cb0"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/195703","number":195703,"mergeCommit":{"message":"Remove
kbn-ace, ace and brace dependencies
(#195703)","sha":"d86ce77217a26747b39ddf240e5703efba1a0cb0"}},{"branch":"8.x","label":"v8.16.0","labelRegex":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v8.16.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove Ace editor in favour of Monaco