-
Notifications
You must be signed in to change notification settings - Fork 574
optionally process comments from tf files as description #565
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
Conversation
da2cd4b to
59beea6
Compare
|
@khos2ow based on our discussion, can you please review. |
You are absolutely correct on this, there shouldn't be any changes in |
|
OK, found the issue. It actually doesn't have anything to do with your implementation, the docs generator is faulty. Can you please add the same property (i.e. |
|
That's exactly where I was looking. Assumed that for options that are not explicitly set, it'll use default. Got it now. Will update. Thanks! |
I'm actually in the process of refactoring |
|
@khos2ow, added config We discussed there might be some overlap with the |
khos2ow
left a comment
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.
Please look at my comments below. Also please update the new config in:
README.mddocs/user-guide/configuration.mddocs/user-guide/configuration/settings.md
This should be good as is and it's not necessary as this is an entirely internal config to load the module. |
|
I realized in no point there's a test with |
|
28d28e4 to
51f63d1
Compare
khos2ow
left a comment
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.
Thank you @poojitha-bikki for all the efforts. lgtm!
would you please now squash your commits into one?
…description fixes issue terraform-docs#552 - process description from comments Signed-off-by: Poojitha Bikki <[email protected]> - fix module tests Signed-off-by: Poojitha Bikki <[email protected]> - optionally read comments for output vars description Signed-off-by: Poojitha Bikki <[email protected]> - set default to true Signed-off-by: Poojitha Bikki <[email protected]> - run make docs Signed-off-by: Poojitha Bikki <[email protected]> - change option name Signed-off-by: Poojitha Bikki <[email protected]> - add option in doc generator; make docs Signed-off-by: Poojitha Bikki <[email protected]> - add config 'ReadComments' Signed-off-by: Poojitha Bikki <[email protected]> - Fix alphabetic order for 'ReadComments' setting Signed-off-by: Poojitha Bikki <[email protected]> - add read-comments in docs Signed-off-by: Poojitha Bikki <[email protected]> - add test for readcomments option Signed-off-by: Poojitha Bikki <[email protected]> - update 'read-comments' flag description Co-authored-by: Khosrow Moossavi <[email protected]> Signed-off-by: Poojitha Bikki <[email protected]>
51f63d1 to
045707b
Compare
|
Thank you @khos2ow for your guidance. |
|
Thank you @poojitha-bikki for the great work! |
Description of your changes
Fixes #552
This PR includes changes to optionally process comments from
.tffiles as description (when description is not provided in terraform inputs, outputs definition)read-commentsI have:
make test.How has this code been tested
TestReadComments()make testTBD
Config setting that maps to the new option addedAdded new config settingread-commentsTests when option is set toset default to true (for backwards compatibility). Added new test cases to test when set tofalse. Currently few tests fail when option is set tofalsefalseandtrue