-
Notifications
You must be signed in to change notification settings - Fork 111
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge branch 'RSDK-9204-experimental' into remove-webcam-logic
- Loading branch information
Showing
126 changed files
with
5,295 additions
and
1,995 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,6 @@ | ||
| # Team SDK is responsible for CLI best practices and wishes to maintain insight into any | ||
| # changes currently being made to the CLI codebase. | ||
| /cli/app.go @viamrobotics/sdk-netcode | ||
| /cli/CONTRIBUTING.md @viamrobotics/sdk-netcode | ||
| /cli/STYLEGUIDE.md @viamrobotics/sdk-netcode | ||
|
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| # Contributing | ||
|
|
||
| Contributions to the Viam CLI are always welcome and appreciated! Every bit helps, and | ||
| credit will always be given. | ||
|
|
||
| ## Types of Contributions | ||
|
|
||
| ### Report Bugs | ||
|
|
||
| If you are reporting a bug, please include: | ||
|
|
||
| - Your operating system name and version. | ||
| - Which version of the viam CLI you are using (viewable with command `viam version`) | ||
| - Any details about your local setup that might be helpful in troubleshooting. | ||
| - The observed behavior. | ||
| - Detailed steps to reproduce the bug. | ||
|
|
||
| ### Fix Bugs | ||
|
|
||
| Look through the GitHub issues for bugs. Anything tagged with "bug" and "help | ||
| wanted" is open to whoever wants to implement it. | ||
|
|
||
| ### Implement Features | ||
|
|
||
| Look through the GitHub issues for features. Anything tagged with "enhancement" | ||
| and "help wanted" is open to whoever wants to implement it. When contributing, please | ||
| consult the [style guide](STYLEGUIDE.md) to ensure consistency within the codebase. | ||
|
|
||
| ### Write Documentation | ||
|
|
||
| You can never have enough documentation! Please feel free to contribute to any | ||
| part of the documentation, such as the official docs, docstrings, or even | ||
| on the web in blog posts, articles, and such. | ||
|
|
||
| ### Submit Feedback | ||
|
|
||
| If you are proposing a feature: | ||
|
|
||
| - Explain in detail how it would work. | ||
| - Keep the scope as narrow as possible, to make it easier to implement. | ||
|
|
||
| ## Get Started! | ||
|
|
||
| Ready to contribute? You can follow the instructions in [README.md](README.md) to install and develop | ||
| the viam CLI tool. | ||
|
|
||
| ## Pull Request Guidelines | ||
|
|
||
| Before you submit a pull request, check that it meets these guidelines: | ||
|
|
||
| 1. The pull request should include additional tests if appropriate. | ||
| 2. If the pull request adds functionality, the docs should be updated. | ||
| 3. The pull request should work for all currently supported operating systems and versions of Python. | ||
|
|
||
|
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,188 @@ | ||
| # Code Style | ||
|
|
||
| Thank you for contributing to the Viam CLI! Please keep in mind the following best practices | ||
| when structuring your code. | ||
|
|
||
| ## Make Arguments Typeful | ||
| Your CLI command's action func should be constructed with `createCommandWithT` and should put | ||
| all command line argument values in a typed struct. This ensures type correctness and reduces | ||
| the risks associated with having contributors manually parse args in each function they write. | ||
|
|
||
| ### Examples | ||
| <details> | ||
| <summary>Good:</summary> | ||
|
|
||
| ```golang | ||
| type fooArgs struct { | ||
| Bar int | ||
| Baz string | ||
| } | ||
|
|
||
| func fooAction(ctx cli.Context, args foo) error { | ||
| bar := args.Bar | ||
| baz := args.Baz | ||
| ... | ||
| } | ||
|
|
||
| ... | ||
|
|
||
| cli.Command{ | ||
| ... | ||
| Flags: []cli.Flag{ | ||
| &cli.StringFlag{ | ||
| Name: Baz, | ||
| }, | ||
| &cli.IntFlag{ | ||
| Name: Bar, | ||
| }, | ||
| }, | ||
| Action: createCommandWithT[fooArgs](fooAction), | ||
| ... | ||
| } | ||
| ``` | ||
| </details> | ||
|
|
||
| <details> | ||
| <summary>Bad:</summary> | ||
|
|
||
| ```golang | ||
| func fooAction(ctx cli.Context) error { | ||
| bar := ctx.Int("Bar") | ||
| baz := ctx.String("Baz) | ||
| ... | ||
| } | ||
| ... | ||
| cli.Command{ | ||
| Flags: []cli.Flag{ | ||
| &cli.StringFlag{ | ||
| Name: BazFlag, | ||
| }, | ||
| &cli.IntFlag{ | ||
| Name: BarFlag, | ||
| }, | ||
| }, | ||
| Action: fooAction, | ||
| ... | ||
| } | ||
| ``` | ||
| </details> | ||
| ## Avoid Flag Bloat | ||
| We have a significant number of flags in the CLI already, many of which (e.g., `organization` or | ||
| `location`) are used in multiple functions. Instead of adding duplicate flags, prefer reusing | ||
| existing flags. If necessary, rename then to indicate their more generic usage. | ||
| ### Example | ||
| <details> | ||
| <summary>Good:</summary> | ||
| ```diff | ||
| -const yourSpecialFlag = "some-cool-flag" | ||
| +const generalSpecialFlag = "some-cool-flag" | ||
| ``` | ||
| </details> | ||
| <details> | ||
| <summary>Bad:</summary> | ||
| ```golang | ||
| const yourSpecialFlag = "some-cool-flag" | ||
| ... | ||
| const mySpecialFlag = "some-cool-flag" | ||
| ``` | ||
| </details> | ||
| ## Hide Help From Commands With Subcommands | ||
| When a parent level command with child commands exists (e.g., `viam organizations` has `list`, | ||
| `logo`, `api-key`, and others as child commands), it makes little sense to show a `help` command | ||
| since the parent level command doesn't do anything on its own. | ||
| ### Example | ||
| <details> | ||
| <summary>Necessary diff:</summary> | ||
| ```diff | ||
| cli.Command{ | ||
| ... | ||
| Name: "my-parent-command", | ||
| + HideHelpCommand: true, | ||
| Subcommands: []*cli.Command{ | ||
| { | ||
| Name: "my-child-command1", | ||
| ... | ||
| }, | ||
| { | ||
| Name: "my-child-command2", | ||
| ... | ||
| }, | ||
| }, | ||
| } | ||
| ``` | ||
| </details> | ||
| ## Create Usage Text With Helper Func, Only Specify Required Args | ||
| When creating usage text for a CLI command, use the `createUsageText` convenience method | ||
| to generate text. Be sure to provide the fully qualified command (less `viam`) as your first | ||
| argument, and only include actually required flags in the `requiredFlags` argument. | ||
| Additionally, be sure to use the `formatAcceptedValues` convenience method for defining usage | ||
| text on a flag where only a discrete set of values are permitted. | ||
| ### Example | ||
| <details> | ||
| <summary>Diff:</summary> | ||
| ```diff | ||
| cli.Command{ | ||
| ... | ||
| Name: "my-parent-command", | ||
| Subcommands: []*cli.Command{ | ||
| Name: "my-child-command", | ||
| Flags: []cli.Flag{ | ||
| &cli.StringFlag{ | ||
| Name: requiredFlag, | ||
| Required: true, | ||
| + Usage: formatAcceptedValues("passes some required value", 'foo', 'bar, 'baz') | ||
| - Usage: "passes some required value. must be either 'foo', 'bar', or 'baz'" | ||
| }, | ||
| &cli.StringFlag{ | ||
| Name: optionalFlag, | ||
| }, | ||
| }, | ||
| + UsageText: createUsageText("my-parent-command my-child-command", []string{requiredFlag}, true, false), | ||
| - UsageText: createUsageText("my-child-command", []string{requiredFlag, optionalFlag}, false, false), | ||
| ... | ||
| } | ||
| } | ||
| ``` | ||
| </details> | ||
| ## Use `DefaultText` Field For Defining Default values | ||
| Instead of adding extra text to the description of what a field does, we should use the automated | ||
| formatting provided by the `DefaultText` field. | ||
| ### Examples | ||
| <details> | ||
| <summary>Good:</summary> | ||
| ```golang | ||
| cli.StringFlag{ | ||
| Name: fooFlag, | ||
| Usage: "sets value of Foo", | ||
| DefaultText: "foo", | ||
| } | ||
| ``` | ||
| </details> | ||
| <details> | ||
| <summary>Bad:</summary> | ||
| ```golang | ||
| cli.StringFlag{ | ||
| Name: fooFlag, | ||
| Usage: "sets value of Foo (defaults to foo)", | ||
| } | ||
| ``` | ||
| </details> |
Oops, something went wrong.