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.

Sign up for GitHub

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

Jump to bottom

Fix minor annoyance of missing '--version' flag from some Help output #1707

Merged
merged 5 commits into from
Sep 30, 2022
Merged

Fix minor annoyance of missing '--version' flag from some Help output #1707

merged 5 commits into from
Sep 30, 2022

Conversation

fnickels
Copy link
Contributor

@fnickels fnickels commented May 24, 2022 •
edited
Loading

In the current code, with a version set, app help does not display the switch option --version, while app --help does.

  • Fix missing '--version' flag from help command
    • with unit tests
  • Add vscode to .gitignore

@CLAassistant
Copy link

CLAassistant commented May 24, 2022 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added the size/M Denotes a PR that chanes 24-99 lines label May 24, 2022
@github-actions
Copy link

github-actions bot commented Sep 7, 2022

The Cobra project currently lacks enough contributors to adequately respond to all PRs. This bot triages issues and PRs according to the following rules:

  • After 60d of inactivity, lifecycle/stale is applied. - After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied and the PR is closed.
    You can:
  • Make a comment to remove the stale label and show your support. The 60 days reset. - If a PR has lifecycle/rotten and is closed, comment and ask maintainers if they'd be interseted in reopening.

@gusega gusega mentioned this pull request Sep 13, 2022
Closed
@gusega
Copy link
Contributor

gusega commented Sep 13, 2022

great that you fixed it @fnickels , I also just noticed this thing. Hope it gets merged soon.

marckhouzam
marckhouzam requested changes Sep 19, 2022
View reviewed changes
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.

This is great @fnickels ! Thanks for fixing this.

Everything looks good except one nit.

.gitignore Outdated Show resolved Hide resolved
@marckhouzam marckhouzam added kind/bug A bug in cobra; unintended behavior area/flags-args Changes to functionality around command line flags and args labels Sep 19, 2022
@marckhouzam marckhouzam added this to the 1.6.0 milestone Sep 19, 2022
@fnickels
Copy link
Contributor Author

fnickels commented Sep 19, 2022 via email

marckhouzam
marckhouzam reviewed Sep 19, 2022
View reviewed changes
command_test.go Outdated Show resolved Hide resolved
@fnickels
Copy link
Contributor Author

fnickels commented Sep 19, 2022 via email

@marckhouzam
Copy link
Collaborator

Should the .idea/ reference also be removed from .gitignore?

There is no official agreement to not put those things in the .gitignore for this project, but I at least didn't want to "pollute" your nice PR with those changes.

marckhouzam
marckhouzam approved these changes Sep 19, 2022
View reviewed changes
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.

LGTM

Thanks @fnickels !

@marckhouzam marckhouzam added the lgtm Denotes "looks good to me" from maintainers and signals other collaboratores that a PR is ready label Sep 19, 2022
@marckhouzam
Copy link
Collaborator

@jpmcb This PR fixes the fact that the --version flag was not output by the help command. I don't think that changing help output is a backwards-compatible issue, so I fee this should go in. Ok with you?

@marckhouzam marckhouzam mentioned this pull request Sep 21, 2022
Merged
jpmcb
jpmcb previously requested changes Sep 29, 2022
View reviewed changes
Copy link
Collaborator

@jpmcb jpmcb left a comment

Choose a reason for hiding this comment

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

Nice! Just a quick touch up:

  • Can you squash your commits and write a single commit message that covers this? (if not, I'd be happy to squash / merge and write one)

@marckhouzam marckhouzam dismissed jpmcb’s stale review September 30, 2022 18:25

I'll address the merging of the commits myself.

@marckhouzam marckhouzam merged commit 7039e1f into spf13:main Sep 30, 2022
@marckhouzam
Copy link
Collaborator

Nice! Just a quick touch up:

  • Can you squash your commits and write a single commit message that covers this? (if not, I'd be happy to squash / merge and write one)

I've gone ahead and done it myself.
Thanks @fnickels for the fix!

@fnickels
Copy link
Contributor Author

fnickels commented Oct 1, 2022

happy to help

jimschubert added a commit to jimschubert/cobra that referenced this pull request Oct 3, 2022
@jimschubert
Merge branch 'main' into doc-slash-commands
b5bc3e3 
* 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)
  ...
@umarcor umarcor mentioned this pull request Oct 3, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marckhouzam marckhouzam marckhouzam approved these changes

@jpmcb jpmcb Awaiting requested review from jpmcb

Assignees
No one assigned
Labels
area/flags-args Changes to functionality around command line flags and args kind/bug A bug in cobra; unintended behavior lgtm Denotes "looks good to me" from maintainers and signals other collaboratores that a PR is ready size/M Denotes a PR that chanes 24-99 lines
Projects
None yet
Milestone
1.6.0
Development

Successfully merging this pull request may close these issues.
5 participants
@fnickels @CLAassistant @gusega @marckhouzam @jpmcb