Skip to content
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

Make main module doc generation optional when generating recursively #621

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

blakegong
Copy link
Contributor

@blakegong blakegong commented Jul 11, 2022

Description of your changes

When using recursive: true, terraform-docs currently generates the doc for ./{OUTPUT_FILE} and ./modules/*/{OUTPUT_FILE}. However, for some pure module repositories, the generation at ./{OUTPUT_FILE} might be unwanted.

In #613, skip-root-readme was suggested as the option name. However, in my opinion, it is the best to name booleans in a non-negative manner to avoid confusion when reading the code, which means skip (or e.g. no-root) should be avoided. Therefore, I went with --recursive-include-main which defaults to true, to preserve existing behaviour.

Closes #613.

I have:

  • Read and followed terraform-docs' [contribution process].
  • All tests pass when I run make test.

How has this code been tested

I tried it on my own TF modules repository, which has a vpc module inside:

➜  terraform-modules ✗ terraform-docs markdown table --recursive --output-file README.md .          
README.md updated successfully
modules/vpc/README.md updated successfully

➜  terraform-modules ✗ terraform-docs markdown table --recursive --recursive-include-main=false --output-file README.md .
modules/vpc/README.md updated successfully

Notice that with --recursive-include-main=false, README.md updated successfully message has disappeared, as it is no longer being generated.

@blakegong
Copy link
Contributor Author

blakegong commented Jul 11, 2022

Oops just realised there are documents to update, will add that. Edit: docs added

Meanwhile, do let me know if this PR looks reasonable to you :)

@pull-request-size pull-request-size bot added size/M and removed size/S labels Jul 11, 2022
@blakegong blakegong changed the title Add option to skip root when generating recursively Make main module doc generation optional when generating recursively Jul 11, 2022
@blakegong
Copy link
Contributor Author

@khos2ow @metmajer could you help take a look? 🙏

@blakegong
Copy link
Contributor Author

@khos2ow 👋 🙏

@blakegong
Copy link
Contributor Author

@khos2ow @metmajer could you help take a look again? 🙏🙏

All the CI runs are green. Just let me know what you think!

@blakegong
Copy link
Contributor Author

@khos2ow @metmajer pinging for reviews 🙏

@miguelaferreira
Copy link

I'm currently unable to run terraform-docs via pre-commit checks because of #613. It would be great if a maintainer would spare some time to review this PR.

@nosrio
Copy link

nosrio commented Feb 10, 2023

Hi! What is missing for this to be merged?

Copy link

@armsnyder armsnyder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a very simple and low risk change. I hope a maintainer sees this and can get it merged.

@@ -42,6 +42,7 @@ terraform-docs asciidoc document [PATH] [flags]
--output-values-from string inject output values from file into outputs (default "")
--read-comments use comments as description when description is empty (default true)
--recursive update submodules recursively (default false)
--recursive-include-main include the main module (default true) (default true)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you don't need to include (default true) in the regular description since then it gets added twice.

@riemers
Copy link

riemers commented Sep 21, 2023

Started 2022, is the project still maintained?

@martinm82
Copy link

Any chance this PR could be merged as this is a great addition to the available functionality.

@DaveKLloyd
Copy link

I would really like his feature also. Will this be merged any time soon?

@kngatineau
Copy link

Would love to see this merged, would be handy!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Proposed
Development

Successfully merging this pull request may close these issues.

When using recursive: true allow skipping changes to root README.md
9 participants