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

Use BasePackage for search output data #529

Merged
merged 3 commits into from
Jun 23, 2020

Conversation

ruflin
Copy link
Contributor

@ruflin ruflin commented Jun 22, 2020

So far the /search output was manually built. This had the risk that the format of the package output and search output diverge but they should not. To prevent this, a BasePackage is introduced. The package is composed out of this BasePackage with additional fields.

This change does not affect the output, only the order of the output.

ruflin added 2 commits June 22, 2020 08:56
So far the `/search` output was manually built. This had the risk that the format of the package output and search output diverge but they should not. To prevent this, a BasePackage is introduced. The package is composed out of this BasePackage with additional fields.

This change does not affect the output, only the order of the output.
@elasticmachine
Copy link

elasticmachine commented Jun 22, 2020

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #529 updated]

  • Start Time: 2020-06-22T07:26:38.393+0000

  • Duration: 10 min 2 sec

Test stats 🧪

Test Results
Failed 0
Passed 105
Skipped 0
Total 105

@ruflin ruflin requested a review from mtojek June 22, 2020 07:37
@ruflin ruflin self-assigned this Jun 22, 2020
@@ -163,28 +163,12 @@ func getPackageOutput(packagesList map[string]map[string]util.Package) ([]byte,
}
sort.Strings(keys)

var output []map[string]interface{}
var output []util.BasePackage

for _, k := range keys {
parts := strings.Split(k, separator)
m := packagesList[parts[0]][parts[1]]
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: is there is a need to check the number of parts here (in case if it's empty)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also stumbled over this line when I did the change and I plan to follow up to make this nicer. There must be a better way here.

@ruflin ruflin merged commit 337a7c5 into elastic:master Jun 23, 2020
@ruflin ruflin deleted the search-package-struct branch June 23, 2020 09:06
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.

3 participants