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

Consolidate all App code into Command #1746

Merged
merged 26 commits into from
Jun 13, 2023
Merged

Conversation

meatballhat
Copy link
Member

@meatballhat meatballhat commented Jun 2, 2023

What type of PR is this?

  • cleanup
  • feature

What this PR does / why we need it:

In addition to what is described in #1586, this change:

  • adds Command.Root() method for returning the "root command" (previously what was App) which is generally used to get the "root Writer". I'd like to make this access more sugary than cCtx.Command.Root().Writer once Context and Command are collapsed 🤞🏼
  • consolidates "running" to a single Run method that accepts only stdlib context.Context and a string slice like os.Args
  • removes the Commands type alias for []*Command
  • adds and uses tracef func for runtime tracing to aid with development, testing, and debugging
  • modifies many (but not all!) tests to use testify/require
  • moves expectFileContent from fish_test.go to cli_test.go and modifies it to use testify/require
  • replaces mutated HelpName string with FullName method, which also removes the ability to customize the name used for a command in help text
  • replaces global variables for some commands and flags with private functions for building copies when needed
  • removes (I assume) accidentally added binary file in ./cmd/
  • probably other notable things 😬 I could have been more careful. Critical feedback wanted! 🙇🏼

Which issue(s) this PR fixes:

Closes #1586

@meatballhat meatballhat added this to the Release 3.x milestone Jun 9, 2023
@meatballhat meatballhat added kind/feature describes a code enhancement / feature request kind/cleanup describes internal cleanup / maintaince area/v3 relates to / is being considered for v3 labels Jun 9, 2023
@meatballhat meatballhat marked this pull request as ready for review June 10, 2023 14:11
@meatballhat meatballhat requested a review from a team as a code owner June 10, 2023 14:11
command.go Outdated Show resolved Hide resolved
context.go Outdated Show resolved Hide resolved
help.go Outdated Show resolved Hide resolved
help.go Outdated Show resolved Hide resolved
dearchap
dearchap previously approved these changes Jun 12, 2023
@meatballhat meatballhat requested review from dearchap and a team June 12, 2023 13:55
@meatballhat meatballhat merged commit d29ceb2 into main Jun 13, 2023
@meatballhat meatballhat deleted the app-command-collapse-again branch June 13, 2023 23:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/v3 relates to / is being considered for v3 kind/cleanup describes internal cleanup / maintaince kind/feature describes a code enhancement / feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consolidate all App code into Command
2 participants