Skip to content

Conversation

@zimeg
Copy link
Member

@zimeg zimeg commented Aug 14, 2025

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
Copy link

codecov bot commented Aug 14, 2025

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.

Copy link
Member Author

@zimeg zimeg left a comment

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 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
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
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
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.

Copy link
Member

@mwbrooks mwbrooks left a comment

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
Member

Choose a reason for hiding this comment

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

🪓 🎉

return err
}
if hasSeeAlso(cmd) {
fmt.Fprintf(buf, "## See also\n\n")
Copy link
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! 🧠

Copy link
Contributor

@lukegalbraithrussell lukegalbraithrussell left a comment

Choose a reason for hiding this comment

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

i'm hyped!

@zimeg
Copy link
Member Author

zimeg commented Aug 15, 2025

@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.

4 participants