Skip to content

fix: generate documentation without template literals and with standard headers#194

Merged
zimeg merged 2 commits into
mainfrom
zimeg-fix-docgen-template
Aug 15, 2025
Merged

fix: generate documentation without template literals and with standard headers#194
zimeg merged 2 commits into
mainfrom
zimeg-fix-docgen-template

Conversation

@zimeg

@zimeg zimeg commented Aug 14, 2025

Copy link
Copy Markdown
Member

Summary

This PR removes template literals and changes headers of generated documentation to match previous build scripts used for upstream pages 📚 ✨

Reviewers

With these changes checked out, confirm expected updates to reference are found:

$ make build && PATH="./bin" slack docgen docs/reference
$ git diff

Requirements

@zimeg zimeg added this to the Next Release milestone Aug 14, 2025
@zimeg zimeg self-assigned this Aug 14, 2025
@zimeg zimeg requested a review from a team as a code owner August 14, 2025 06:25
@zimeg zimeg added bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented docs M-T: Documentation work only semver:patch Use on pull requests to describe the release version increment labels Aug 14, 2025
@codecov

codecov Bot commented Aug 14, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 39.31624% with 71 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.90%. Comparing base (206203d) to head (afce3b3).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
cmd/docgen/docgen.go 39.31% 59 Missing and 12 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #194      +/-   ##
==========================================
- Coverage   63.07%   62.90%   -0.17%     
==========================================
  Files         212      212              
  Lines       21672    21782     +110     
==========================================
+ Hits        13670    13703      +33     
- Misses       6953     7013      +60     
- Partials     1049     1066      +17     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@zimeg zimeg left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

💌 Notes on updates follow for the kind reviewers but also please find in an expected diff:

  • "Synopsis" is changed to "Description"
  • "Options" is changed to "Flags"
  • "Options inherited from parent commands" is changed to "Global flags"
  • "SEE ALSO" is changed to "See also"

Comment thread cmd/docgen/docgen.go
Comment on lines +180 to +186
if len(cmd.Example) > 0 {
fmt.Fprintf(buf, "## Examples\n\n")
fmt.Fprintf(buf, "```\n%s\n```\n\n", cmd.Example)
}
if err := genMarkdownCommandFlags(buf, cmd); err != nil {
return err
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

👁️‍🗨️ note: This ordering might be updated in a follow up PR to match --help outputs.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

📣 note: This change is made in #195!

Comment on lines -377 to +378
Remediation: fmt.Sprintf("Choose a specific app with %s", style.Highlight("--app <app_id>")),
Remediation: "Choose a specific app with `--app <app_id>`",

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

⚠️ note: Backticks are preferred to Highlight since styles are not found in these error messages - the error map is compiled without styles toggled "on".

🐣 note: This matches the fallback behavior of Commandf and ought not cause problems in output. An example command might use "--app ..." as an option to show this.

@mwbrooks mwbrooks left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

✅ Beaaaauty! Love this change, thank you!

🧪 Tested locally and all the changes are either expected or improvements (updating stale content).

Comment on lines -70 to -76
// Cobra are a group of Cobra functions shared by all packages and enables tests & mocking
Cobra struct {
// GenMarkdownTree defaults to `doc.GenMarkdownTree(...)` and can be mocked to test specific use-cases
// TODO - This can be moved to cmd/docs/docs.go when `NewCommand` returns an instance of that can store `GenMarkdownTree` as
// a private member. The current thinking is that `NewCommand` would return a `SlackCommand` instead of `CobraCommand`
GenMarkdownTree func(cmd *cobra.Command, dir string) error
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🪓 🎉

Comment thread cmd/docgen/docgen.go
return err
}
if hasSeeAlso(cmd) {
fmt.Fprintf(buf, "## See also\n\n")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ignore-me: While I prefer "Title Case", this looks correct because our docs use "Sentence case". I think it's best for the CLI to stay aligned with our docs. So please ignore me! 🧠

@lukegalbraithrussell lukegalbraithrussell left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i'm hyped!

@zimeg

zimeg commented Aug 15, 2025

Copy link
Copy Markdown
Member Author

@mwbrooks @lukegalbraithrussell Appreciate the kind reviews so much. I'm excited for all the more customizations possible from these changes. But for now let's merge this. 🚢 💨

@zimeg zimeg merged commit 2de192d into main Aug 15, 2025
6 checks passed
@zimeg zimeg deleted the zimeg-fix-docgen-template branch August 15, 2025 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented docs M-T: Documentation work only semver:patch Use on pull requests to describe the release version increment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants