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

[WIP] refactor volumes to demonstrate how globals could be avoided #66

Closed
wants to merge 1 commit into from

Conversation

displague
Copy link
Member

@displague displague commented Jul 22, 2020

This experimental refactor of the volumes commands intends to demonstrate that global variables can be avoided with spf13 usage.

Testing these commands should be simpler because the argument and client variables are not set in a global context.

API mocks have been but these are not currently being used by tests (something that I would want to add to validate this approach).

Some of the commands have lost their json and yaml flags in this initial state. I'm interested in seeing if these arguments can become global or if they are best suited for per-command-action use.

Relates to #33


Observations and Ponderances:

  • this PR does not break the cli, only volumes are refactored and the rest of the project is mostly untouched
  • cmd/*volume*.go moves to a volume package in internal/volumes/.
  • When everything else has moved to internal/, cmd/packet can be created, which would facilitate Document go get install method #53. go get would then create binaries with the desired packet name, rather than packet-cli.
  • Each command is explicitly registered, similar to what was is done currently in init() calls.
  • output package and functions are created as a way to preserve the existing output functions while moving commands to their own package.
  • Stderr is not used for error messages, neither is spf13's RunE return error codes when errors are encountered #70 error messages duplicate and error code is still not reported #75
  • license headers and go function and constant descriptions have not been added.
  • I took advantage of spf13's Example field, rather than using the Long field. I can see why Long was used to portray examples, and I think this was to avoid the short description from being repeated in the help and generated markdown. I think a better experience can be had by providing meaningful content in the long description fields.

@displague displague added the enhancement New feature or request label Jul 22, 2020
@displague displague mentioned this pull request Jul 22, 2020
@displague
Copy link
Member Author

All of the features presented in this draft have been incorporated, most notably by #133. Closing.

@displague displague closed this Dec 13, 2021
@displague displague deleted the less-globals branch December 13, 2021 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant