-
Notifications
You must be signed in to change notification settings - Fork 38
fix: create script to generate config doc #3721
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
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.
Code Review
This pull request introduces a script to automatically generate documentation for the librarian.yaml configuration schema, which is a great addition for maintainability. The script parses the Go structs in internal/config and produces a markdown file.
My review focuses on improving the robustness and performance of the new generator script, in line with the TODO to clean up the logic. I've suggested two main changes:
- Making the logic for identifying internal structs more reliable by using the parsed struct map instead of a name-based heuristic.
- Improving performance by pre-compiling a regular expression.
These changes will make the generator more accurate and efficient. The generated documentation itself looks good and will be a valuable resource.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3721 +/- ##
==========================================
+ Coverage 82.43% 82.78% +0.35%
==========================================
Files 138 138
Lines 12758 13038 +280
==========================================
+ Hits 10517 10794 +277
- Misses 1747 1751 +4
+ Partials 494 493 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
/gemini review |
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.
Code Review
This pull request introduces a valuable script to automatically generate configuration schema documentation from Go source files, which will help keep documentation synchronized with the code. The implementation is solid and includes a good set of unit tests. My review includes a high-severity fix for a bug in markdown link generation for embedded structs, along with a medium-severity suggestion to improve error handling.
|
Also would be great if you can create several files to split the logic you are adding 😃 |
|
Thanks @miguelvelezsa for your reviews!
I'd prefer to keep this code in one file, at least for now: 1)this is an internal tool for generating one specific doc file, it is more self-contained, and less likely for its code to be reused in other parts of the project. 2) it is relatively a small tool not too hard to read (I hope) at the moment. |
Add script to generate a doc/config-schema.md that describes config schema based off
internal/config/config.goand other referenced configs. Ordered to list Structs from config.go at the top, and others alphabetically.This tool must be package main to provide the entry point for the Go compile, as it's purpose is to be "run" (either manually or via go generate).
Manual verification:
The documentation can be regenerated using the standard Go generate tool:
go generate ./internal/config/...Alternatively, for manual runs from the root:
go run -tags configdocgen cmd/config_doc_generate.goFix #3681