Skip to content

Add search to the elements in dahsboards that can be used by value#635

Merged
jsoriano merged 1 commit intoelastic:mainfrom
jsoriano:search-by-value
Oct 5, 2023
Merged

Add search to the elements in dahsboards that can be used by value#635
jsoriano merged 1 commit intoelastic:mainfrom
jsoriano:search-by-value

Conversation

@jsoriano
Copy link
Copy Markdown
Member

@jsoriano jsoriano commented Oct 5, 2023

Fix #633

@jsoriano jsoriano requested a review from kertal October 5, 2023 07:17
@jsoriano jsoriano requested a review from a team as a code owner October 5, 2023 07:17
@jsoriano jsoriano self-assigned this Oct 5, 2023
@elasticmachine
Copy link
Copy Markdown

💚 Build Succeeded

cc @jsoriano

@jsoriano jsoriano mentioned this pull request Oct 5, 2023
Copy link
Copy Markdown
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.

LGTM, looking at the code change, just a question how it could be tested, since this is new topic to me

var references []reference
for _, reference := range allReferences {
switch reference.Type {
case "lens", "visualization", "map":
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, just out of curiosity , no being where this warning is triggered, how can this be tested? thx

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

You can run the following command to test from a package directory:

elastic-package check

You can find packages to test in https://github.com/elastic/integrations/

These errors will be displayed only as warnings, unless you set the environment variable PACKAGE_SPEC_WARNINGS_AS_ERRORS=true.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

thx!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just as a note, this is going to be applied independently of the spec version used in the packages.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Just as a note, this is going to be applied independently of the spec version used in the packages.

I think this is fine, as it will only produce validation errors in v3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Change Proposal] Add "search" to validate_visualizations_used_by_value

4 participants