Skip to content

Rearrange cli commands #272

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

Merged
merged 8 commits into from
Jul 17, 2019
Merged

Rearrange cli commands #272

merged 8 commits into from
Jul 17, 2019

Conversation

masci
Copy link
Contributor

@masci masci commented Jul 11, 2019

This PR has no functional changes, it only rearrange the layout of the various command packages so that dependencies are cleaner, in particular:

  1. cli package depends on all the packages implementing a command
  2. None of the command packages depends on cli anymore
  3. None of the root packages depends on cli anymore, except for main
  4. output was removed from the root and added under cli since it's only used there

The new layout is this

cli
├── board
├── cli.go
├── cli_test.go
├── compile
├── config
├── core
├── daemon
├── errorcodes (to be used with os.Exit)
├── generatedocs
├── globals (for data that needs to be accessible to any command)
├── instance (common core operations used by different commands)
├── lib
├── output (common output operations, include contents of the old package at the root)
├── sketch
├── testdata
├── upload
└── version

PR also includes some minor changes to remove unneeded structs and indirection levels, as well as renaming few functions for clarity.

Tests were marked as Integration because they all interact with the actual repos. In the effort of keeping exported symbols at the bare minimum, some were un-exported but tests are now part of the same cli package, I think this use case is legit.

@masci masci marked this pull request as ready for review July 11, 2019 08:33
@mastrolinux mastrolinux merged commit 3d797f5 into master Jul 17, 2019
@masci masci deleted the massi/monitor branch July 17, 2019 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants