-
Notifications
You must be signed in to change notification settings - Fork 4.9k
Add Docs for document-fmt #30848
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
base: main
Are you sure you want to change the base?
Add Docs for document-fmt #30848
Conversation
@@ -0,0 +1,58 @@ | |||
## October 14, 2025 (TBD) |
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.
Not sure we really need a changelog, we don't have any for our other internal tools
./scripts/documentfmt-fix.sh | ||
``` | ||
|
||
### Command-Line Options |
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.
Instead of listing all options, and us having to then keep this document up-to-date, we should probably just remove this section and direct users to run go run ./internal/tools/document-fmt/main.go --help
for available commands and options. 99% of the time usage will be limited to running the make targets anyway
if fix { | ||
// Create and add the missing section | ||
section = &markdown.ExampleSection{} | ||
// Set default content... | ||
d.Document.Sections = append(d.Document.Sections, section) | ||
d.Document.HasChange = true |
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.
While this is an example, since it's included it should be updated to be accurate:
- this isn't setting any content because it didn't template it or call SetContent
- appending a section directly is really only valid for the API version section as that appears last in the documentation. For things like an Example section (or other sections we expect in a certain order) there are helpers to use. E.g.
markdown.InsertAfterSection
ormarkdown.InsertBeforeSection
If certain resources need to skip a rule, add exceptions in `rule/rule_exceptions.go`: | ||
|
||
```go | ||
func shouldSkipRule(resourceName string, ruleID string) bool { |
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.
This isn't a function that exists and also not how exceptions are added (Exceptions
is a global map var with a nested map so we can access without having to loop)
1. Create section implementation in `markdown/section_*.go` | ||
2. Implement the `Section` interface | ||
3. Register the section type in `markdown/registration.go` | ||
4. Add template if needed in `template/template.go` |
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.
section templates don't go here
|
||
## Additional Notes | ||
|
||
- The tool uses the `afero` filesystem abstraction for testability |
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.
It uses afero
simply because it was easy to use for filesystem access, not for testability. I don't think this should be included in the readme
## Integration with CI/CD | ||
|
||
The tool integrates with the repository's CI/CD pipeline: | ||
|
||
- **Pre-commit**: Can be run locally before committing | ||
- **CI Validation**: `scripts/documentfmt-validate.sh` runs in CI to catch issues | ||
- **Automated Fixing**: Developers can run `make document-fix` to auto-correct issues | ||
- **Exit Codes**: Non-zero exit code on validation failure stops the build | ||
|
||
## Additional Notes | ||
|
||
- The tool uses the `afero` filesystem abstraction for testability | ||
- All rules can operate in both validate-only and fix modes | ||
- The validator maintains state across all resources being processed | ||
- Template rendering uses Go's `text/template` package for flexibility | ||
- Color-coded output helps distinguish errors, warnings, and success messages |
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.
Are these 2 sections needed? I'm not sure it's adding much value to anyone using or modifying the tool and it contains some inaccurate + some redundant information
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.
I'm fine leaving them out.
Co-authored-by: sreallymatt <[email protected]>
Community Note
Description
PR Checklist
For example: “
resource_name_here
- description of change e.g. adding propertynew_property_name_here
”Changes to existing Resource / Data Source
Testing
Change Log
Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.
azurerm_resource
- support for thething1
property [GH-00000]This is a (please select all that apply):
Related Issue(s)
Fixes #0000
AI Assistance Disclosure
Rollback Plan
If a change needs to be reverted, we will publish an updated version of the provider.
Changes to Security Controls
Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.
Note
If this PR changes meaningfully during the course of review please update the title and description as required.