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

[Visualizations] Removes the visualize:enableLabs advanced setting #157699

Merged
merged 9 commits into from
May 22, 2023
Merged

[Visualizations] Removes the visualize:enableLabs advanced setting #157699

merged 9 commits into from
May 22, 2023

Conversation

stratoula
Copy link
Contributor

@stratoula stratoula commented May 15, 2023 •
edited
Loading

Summary

Closes #152833

This PR:

  • Removes the visualize:enableLabs setting which doesn't do anything after the presentation team decided to hide the creation of the legacy input controls from the UI
  • Cleanups wherever the components associated with this feature were used

@github-actions
Copy link
Contributor

Documentation preview:

@stratoula stratoula added Feature:Visualizations Generic visualization features (in case no more specific feature label is available) release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting v8.9.0 labels May 16, 2023
@stratoula stratoula changed the title Remove the visualize:enableLabs setting [Visualizations] Removes the visualize:enableLabs advanced setting May 16, 2023
@stratoula stratoula marked this pull request as ready for review May 16, 2023 11:14
@stratoula stratoula requested review from a team as code owners May 16, 2023 11:14
@stratoula stratoula added the Team:Visualizations Visualization editors, elastic-charts and infrastructure label May 16, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-visualizations @elastic/kibana-visualizations-external (Team:Visualizations)

@lukeelmers
Copy link
Member

Be sure to add a migration to core to remove this from the underlying saved objects 🙂
https://github.com/elastic/kibana/blob/main/packages/core/ui-settings/core-ui-settings-server-internal/src/saved_objects/migrations.ts

@stratoula
Copy link
Contributor Author

Thanx @lukeelmers I didnt know about it! Adding this now

@stratoula stratoula requested a review from a team as a code owner May 16, 2023 14:02
@stratoula
Copy link
Contributor Author

@elasticmachine merge upstream

@stratoula
Copy link
Contributor Author

stratoula commented May 16, 2023 •
edited
Loading

@elastic/kibana-core I dont get why this jest test fails, the version that I should use on the migration is the current one which is 8.9 right?

[Error: Invalid migration for type config. Property '8.9.0' cannot be greater than the current Kibana version '8.8.0'.]

@afharo afharo mentioned this pull request May 16, 2023
Closed
2 tasks
stratoula
stratoula commented May 16, 2023
View reviewed changes
src/core/server/integration_tests/elasticsearch/version_compatibility.test.ts
@@ -85,7 +85,8 @@ describe('Version Compatibility', () => {
await expect(startServers({})).resolves.toBeUndefined();
});

it('should start when ES is next minor', async () => {
// skipping this as it fails when a migration is added
it.skip('should start when ES is next minor', async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Skipping this after discussion with @afharo

Copy link
Member

Choose a reason for hiding this comment

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

Created #157901 for us to figure out how to reenable it.

@afharo afharo mentioned this pull request May 16, 2023
Open
@Heenawter Heenawter self-requested a review May 16, 2023 17:21
afharo
afharo approved these changes May 17, 2023
View reviewed changes
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.

LGTM

sebelga
sebelga approved these changes May 17, 2023
View reviewed changes
Copy link
Contributor

@sebelga sebelga left a comment

Choose a reason for hiding this comment

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

LGTM

Heenawter
Heenawter approved these changes May 17, 2023
View reviewed changes
Copy link
Contributor

@Heenawter Heenawter left a comment

Choose a reason for hiding this comment

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

Code owners review + did a quick local test to ensure that the legacy controls still function as expected. Excited to be making progress in removing the legacy controls for good! 🎉

drewdaemon
drewdaemon approved these changes May 18, 2023
View reviewed changes
Copy link
Contributor

@drewdaemon drewdaemon left a comment

Choose a reason for hiding this comment

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

Makes sense to me. Code review only

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
visualizations 337 335 -2

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
visualizations 776 774 -2

Async chunks

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

id before after diff
dashboard 436.4KB 436.4KB -82.0B
visualizations 260.2KB 259.9KB -283.0B
total -365.0B

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
visualizations 19 18 -1

Page load bundle

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

id before after diff
visualizations 58.1KB 56.6KB -1.6KB
Unknown metric groups

API count

id before after diff
visualizations 806 804 -2

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
securitySolution 400 404 +4
total +6

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
securitySolution 480 484 +4
total +6

History

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

@stratoula stratoula merged commit 363f2b5 into elastic:main May 22, 2023
delanni pushed a commit to delanni/kibana that referenced this pull request May 25, 2023
@stratoula @kibanamachine @delanni
[Visualizations] Removes the visualize:enableLabs advanced setting (e…
1a2e008 
…lastic#157699)

## Summary

Closes elastic#152833

This PR:

- Removes the visualize:enableLabs setting which doesn't do anything
after the presentation team decided to hide the creation of the legacy
input controls from the UI
- Cleanups wherever the components associated with this feature were
used

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@drewdaemon drewdaemon drewdaemon approved these changes

@sebelga sebelga sebelga approved these changes

@afharo afharo afharo approved these changes

@Heenawter Heenawter Heenawter approved these changes

Assignees
No one assigned
Labels
backport:skip This commit does not require backporting Feature:Visualizations Generic visualization features (in case no more specific feature label is available) release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.9.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Visualizations] Remove the visualize:enableLabs setting
9 participants
@stratoula @elasticmachine @lukeelmers @kibana-ci @drewdaemon @sebelga @afharo @Heenawter @kibanamachine