Reorganize comments in package templates#2869
Conversation
|
test integrations |
⏳ Build in-progress, with failures
Failed CI StepsHistorycc @jsoriano |
|
Created or updated PR in integrations repository to test this version. Check elastic/integrations#15153 |
mrodm
left a comment
There was a problem hiding this comment.
LGTM!
Just one question about a missing input in the generated README of a test package.
| * [logfile](https://www.elastic.co/docs/reference/integrations/filestream) | ||
| * [apache/metrics](https://www.elastic.co/docs/reference/beats/metricbeat/metricbeat-metricset-apache-status) | ||
| <details> | ||
| <summary>httpjson</summary> |
There was a problem hiding this comment.
It looks like it is not related to the changes related to this PR, but is it expected that logfile input is missing here ?
Should it be added to internal/docs/_static/inputs?
There was a problem hiding this comment.
shall we add info for the missing inputs?
yeah good catch, I think I got logfile confused with (deprecated) log when compiling the original list and we skipped it
@jsoriano did you want to add it here or should I open a separate PR?
Btw, when we add or update input files the docs for packages using them will need an update and CI will fail, we will need to see how to handle this when we start modifying these files.
right, this is something we didn't quite consider. we knew it would be difficult to update all packages, but not that it would temporarily break CI
@mjwolf maybe this is an issue docs phase 2 can also consider? it essentially relates back to the final bullet point Common documentation sections must be manually duplicated across integrations, making updates require mass manual changes across the entire portfolio.
There was a problem hiding this comment.
shall we add info for the missing inputs?
yeah good catch, I think I got logfile confused with (deprecated) log when compiling the original list and we skipped it
@jsoriano did you want to add it here or should I open a separate PR?
Umm, it is linking to filestream, but it seems to be actually using the deprecated log input, so maybe we are fine if we don't want to document deprecated inputs?
Though I think there are many integrations still using this input.
There was a problem hiding this comment.
maybe we don't add it to the selections options when users are making a new datastream, but we still add it so existing integrations can have the documentation piece?
There was a problem hiding this comment.
Yes, that would be a good option. Could you provide the files for logfile?
There was a problem hiding this comment.
Yes! Will have something by the end of the week
| "indent": indent, | ||
| "yamlString": packages.VarValueYamlString, | ||
| } | ||
| t := template.Must(template.New("template").Funcs(funcs).Delims("{[", "]}").Parse(templateBody)) |
There was a problem hiding this comment.
Nice to use different delimiters when creating and building packages 👍
|
I am going to merge this to fix the issues with existing packages. @kgeller, please prepare the files for |
Follow up to #2826 and #2707, fixes issues found in https://buildkite.com/elastic/integrations/builds/30790.
Reorganize templating in archetype templates so different delimiters are used depending on the stage. This helps rendering placeholders for the next step, and helps hiding comments that are not relevant in the files included in built packages.
The delimiters used are:
{[/]}.{{/}}.{{/*/*/}}are used for comments that are intended for developers.In addition:
inputDocsincludes the header, so it can be used to generate content only if there is any input.generatedHeaderis added, that can be used in template docs files to include the header about generated files. It is added to README template for new packages.