-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Generate Docs fo Dynamic Configs #8861
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
|
Sample output: |
| }, | ||
| }, | ||
| { | ||
| Name: "config", |
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.
Integrate this with the validate-dynamic-config command above (probably move that one into here)
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'll pull in render-config as well
| Key() Key | ||
| Precedence() Precedence | ||
| Validate(v any) error | ||
| Documentation() SettingDoc |
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.
There are already getters for Key and Precedence, let's just add new individual getters for description and default value instead of a new struct
| settingType struct { | ||
| Name string | ||
| GoType string | ||
| TypeName string // lowercase type name for documentation |
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.
What's the value in having an extra value here over just using the Go type via reflection?
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.
forgot why I added it, I'll see if i can remove it
| return result | ||
| } | ||
|
|
||
| // getTypeName returns a human-readable type name for documentation. |
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.
is all this complexity worth it instead of just fmt.Sprintf("%T", v)?
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'll see if I can remove it.
|
will open another PR after adding the ability for tdbg to pull these configs. |
What changed?
Describe what has changed in this PR.
Why?
Tell your future self why have you made these changes.
How did you test it?
Potential risks
Any change is risky. Identify all risks you are aware of. If none, remove this section.