Fix packaging of OSS light modules#15957
Conversation
|
run beats-ci/package |
sayden
left a comment
There was a problem hiding this comment.
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 packageRelated issues