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

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

Merged
merged 1 commit into from
Oct 5, 2023
Merged

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

merged 1 commit into from
Oct 5, 2023

Conversation

jsoriano
Copy link
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

💚 Build Succeeded

cc @jsoriano

@jsoriano jsoriano mentioned this pull request Oct 5, 2023
Closed
kertal
kertal approved these changes Oct 5, 2023
View reviewed changes
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.

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

code/go/internal/validator/semantic/validate_visualizations_used_by_value.go
@@ -83,7 +83,7 @@ func anyReference(val interface{}) ([]reference, error) {
var references []reference
for _, reference := range allReferences {
switch reference.Type {
case "lens", "visualization", "map":
Copy link
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
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
Member

Choose a reason for hiding this comment

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

thx!

Copy link
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
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.

@jsoriano jsoriano merged commit 653ab9d into elastic:main Oct 5, 2023
@jsoriano jsoriano deleted the search-by-value branch October 5, 2023 14:22
@drewdaemon drewdaemon mentioned this pull request Nov 9, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrodm mrodm mrodm approved these changes

@kertal kertal kertal approved these changes

Assignees

@jsoriano jsoriano

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Change Proposal] Add "search" to validate_visualizations_used_by_value
4 participants
@jsoriano @elasticmachine @kertal @mrodm