Use .zip packages by default in Elastic stack #886
Conversation
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
🤖 GitHub commentsTo re-run your PR in the CI, just comment with:
|
This reverts commit 035a7c5.
🌐 Coverage report
|
| // shouldDirectoryBeSkipped function checks if absolute path or last element should be skipped. | ||
| func shouldDirectoryBeSkipped(path string, skippedDirs []string) bool { | ||
| for _, d := range skippedDirs { | ||
| if name == d { | ||
| if path == d || filepath.Base(path) == d { | ||
| return true | ||
| } | ||
| } |
There was a problem hiding this comment.
As a suggestion, skippedDirs parameter could be changed to be of type map[string]bool and just look whether or not the key is present. That would avoid to compare all the elements for every element in the directory when executing CopyWithSkipped.
It's also true that this would just have some benefits in the boot process (BootUp function) according to the current usages of this function, so not sure if it can be kept just an array for now. WDYT ?
Some data, with the tests I've being doing with metrics I saw that there are at least 1584 packages being indexed currently.
func shouldDirectoryBeSkipped(path string, skippedDirs map[string]bool) bool {
if _, found := skippedDirs[path]; found {
return true
}
if _, found := skippedDirs[filepath.Base(path)]; found {
return true
}
return false
}There was a problem hiding this comment.
Keep in mind that the real use case for this is building one package at a time and including it in the stack. Also, the skippedDirs list will contain max. 1-2 entries on average, so I would keep the array here.
Let me know what you think :)
There was a problem hiding this comment.
Ah! Ok, I thought that the BootUp function was related to walk through all the packages and versions. If that is intended for just one package with just a few entries, then let's just keep the array of directories 👍
3 participants
Fixes: #584