-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Fix packaging of OSS light modules #15957
Conversation
run beats-ci/package |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Let's move this forward but I'd like a follow up PR where this is polished a bit.
Functions names like prepareLightModules(path... string) error
are a bit obscure at first sight. Specially when its comment states generate
wich is not the same than prepare
and in both cases are vague descriptions about what the function is doing. If "preparation" is about "cleaning", "creating folders", "going through paths for something" and then executing tasks... this is a lot to describe it with prepare
.
Same with calls like devtools.WithModulesD
or lightModulesPackaging()
. The latter is particularly obscure because it uses a global variable, which implies that testing is going to be tricky. Why it cannot be passed as dependency? Don't answer me, it's a rethorical question 😄 I mean that it must be clear in the code and/or comments
Just trying to be constructive 😉
@sayden thanks! I have renamed some helpers to be more consistent with the naming in the rest of magefiles. There are still some things that could be refactored apart of the ones you mention, for example Regarding the use of |
run beats-ci/package |
Umm, I have merged it (2cde4ce) but the issue hasn't been updated, closing it. |
What does this PR do?
Move light modules packaging code in Metricbeat to common methods for X-Pack and OSS, so they are properly packaged in both cases.
Add a check to OSS metricbeat packaging test to check that it includes at least one light module.
Why is it important?
Metricbeat 7.6.0 includes OSS light modules, but they are not being packaged.
Checklist
I have made corresponding changes to the documentationI have made corresponding change to the default configuration filesHow to test this PR locally
Packages can be built with
mage package
Related issues