bitnami / readme-generator-for-helm

Auto generate READMEs for Helm Charts
https://bitnami.com
Apache License 2.0
221 stars 48 forks source link

Add support for section description above parameter table #46

Closed justusbunsi closed 2 years ago

justusbunsi commented 2 years ago

Description of the change

As of now there is no possibility of adding descriptive text above a parameter table in a section. The suggested changes add exactly this by adding two new tags to config.json: descriptionStart and descriptionEnd. Between both tags a user can provide markdown flavored text that will be added above a parameter table inside a section. It supports one description for each section.

Benefits

Users get a better understanding for various parameter use cases and are provided with cross references to external documentation. This depends on the maintainers' decision. πŸ™‚

Additional information

I had some difficulties detecting the actual end of auto-generated README content as the generator did not expect anything except tables or headlines in the auto-generated README part. Instead of looking for the first non-table or non-headline line beginning at the top, it seemed more practical to search for the last table-like line beginning at the end of the parameters section. That way all text blocks between tables are treated as auto-generated content and properly replaced as such.

Additional tests ensure correct behavior for different structures.

Checklist

Signed-off-by: Steven Kriegler 61625851+justusbunsi@users.noreply.github.com

justusbunsi commented 2 years ago

@dgomezleon thanks for the feedback. Will apply it later today. What I am thinking about right now: The default configuration includes both new tags. What about customized configurations? For those users it would be a breaking change as the regex building would fail due to missing keys. Should I integrate a fail safe to not make it a breaking change? Or did I miss a merge of default config with the custom provided one?

justusbunsi commented 2 years ago

@miguelaeh Thanks for your review. I think I found a way to distinguish both parameter and section inside the result of parseMetadataComments. Therefore, I introduced two new objects: Section and Metadata. The Metadata object is a wrapper object that will be returned by this function instead of a plain array. It contains both sections and parameters as different arrays and allows for further data processing to access their required data structure.

What I am thinking about right now: The default configuration includes both new tags. What about customized configurations? For those users it would be a breaking change as the regex building would fail due to missing keys. Should I integrate a fail safe to not make it a breaking change? Or did I miss a merge of default config with the custom provided one?

Is this something that I have to take care of?

dgomezleon commented 2 years ago

@miguelaeh Thanks for your review. I think I found a way to distinguish both parameter and section inside the result of parseMetadataComments. Therefore, I introduced two new objects: Section and Metadata. The Metadata object is a wrapper object that will be returned by this function instead of a plain array. It contains both sections and parameters as different arrays and allows for further data processing to access their required data structure.

What I am thinking about right now: The default configuration includes both new tags. What about customized configurations? For those users it would be a breaking change as the regex building would fail due to missing keys. Should I integrate a fail safe to not make it a breaking change? Or did I miss a merge of default config with the custom provided one?

Is this something that I have to take care of?

Thanks for all the changes @justusbunsi.

Could you please update the readme explaining this situation for those using custom configs?

justusbunsi commented 2 years ago

@miguelaeh I've added a note regarding customized configuration files. It seems to be non-breaking for the end user unless someone uses the @undefined key for some of the tags. If a key is missing in the config.json, the regex for that identifier would be /^\s*#\s*undefined\s*(.*)/, which might collide with the intended @undefined. But I guess this is far too rare to be a breaking change. πŸ˜†

dgomezleon commented 2 years ago

Thank you @justusbunsi.

We have merged it.

justusbunsi commented 2 years ago

Awesome. Thanks for helping me getting this finished.