Skip to content
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

Clarify SetContext documentation #1748

Merged
merged 1 commit into from Aug 30, 2022
Merged

Clarify SetContext documentation #1748

merged 1 commit into from Aug 30, 2022

Conversation

katexochen
Copy link
Contributor

Documentation of Context() was fixed in #1639, especially the fact that the underlying ctx is set to context.Background when Execute() is called (otherwise returns nil).

The documentation of SetContext() states that the ctx is set to context.Background "by default", which is a bit imprecise (and confused some coworker of mine). I think the Context() method provides the needed information when exactly the context is set to Background. I don't see a need to also document this behavior in the comment of SetContext(), as it is only interesting for a caller of Context().

@github-actions github-actions bot added the size/XS Denotes a PR that changes 0-9 lines label Jul 2, 2022
Copy link
Collaborator

@marckhouzam marckhouzam left a comment

Choose a reason for hiding this comment

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

I agree that the mention of context.Background here is more confusing that valuable.

@marckhouzam marckhouzam merged commit 2a7647f into spf13:main Aug 30, 2022
@marckhouzam marckhouzam added this to the 1.6.0 milestone Aug 30, 2022
@marckhouzam marckhouzam added area/cobra-command Core `cobra.Command` implementations kind/documentation Documentation of cobra itself labels Aug 30, 2022
@katexochen katexochen deleted the fix/setcontext-comment branch August 30, 2022 08:54
jimschubert added a commit to jimschubert/cobra that referenced this pull request Oct 3, 2022
* main: (39 commits)
  Add '--version' flag to Help output (spf13#1707)
  Expose ValidateRequiredFlags and ValidateFlagGroups (spf13#1760)
  Document option to hide the default completion cmd (spf13#1779)
  ci: add workflow_dispatch (spf13#1387)
  add missing license headers (spf13#1809)
  ci: use action/setup-go's cache (spf13#1783)
  Adjustments to documentation (spf13#1656)
  Rename Powershell completion tests (spf13#1803)
  Support for case-insensitive command names (spf13#1802)
  Deprecate ExactValidArgs() and test combinations of args validators (spf13#1643)
  Use correct stale action `exempt-` yaml keys (spf13#1800)
  With go 1.18, we must use go install for a binary (spf13#1726)
  Clarify SetContext documentation (spf13#1748)
  ci: test on Golang 1.19 (spf13#1782)
  fix: show flags that shadow parent persistent flag in child help (spf13#1776)
  Update gopkg.in/yaml.v2 to gopkg.in/yaml.v3 (spf13#1766)
  fix(bash-v2): activeHelp length check syntax (spf13#1762)
  fix: correct command path in see_also for YAML doc (spf13#1771)
  build(deps): bump github.com/inconshreveable/mousetrap (spf13#1774)
  docs: add zitadel to the list (spf13#1772)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cobra-command Core `cobra.Command` implementations kind/documentation Documentation of cobra itself size/XS Denotes a PR that changes 0-9 lines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants