-
Notifications
You must be signed in to change notification settings - Fork 3
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
sc-235696/list projects #20
Conversation
This pull request has been linked to Shortcut Story #235696: create command to list existing projects. |
"u", | ||
"http://localhost:3000", | ||
"LaunchDarkly base URI.", | ||
"accessToken", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alphabetized these.
func ListProjects(ctx context.Context, client2 Client) ([]byte, error) { | ||
projects, err := client2.List(ctx) | ||
if err != nil { | ||
// 401 - should return unauthorized type error with body(?) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll follow up with a PR to handle errors.
|
||
type MockClient struct{} | ||
|
||
func (c MockClient) List(ctx context.Context) (*ldapi.Projects, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll change this once we have better error handling.
cmd/projects.go
Outdated
viper.GetString("accessToken"), | ||
viper.GetString("baseUri"), | ||
) | ||
response, err := projects.ListProjects( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By separating the command from the actual call, it makes it simpler to test. I could see not needing to test the command if it's responsibility is only to get the flag values and set up whatever the domain (in this case projects.ListProjects
needs.
cmd/projects.go
Outdated
|
||
func runProjectsGet(cmd *cobra.Command, args []string) error { | ||
// TODO: handle missing flags | ||
if viper.GetString("accessToken") == "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think cobra has a way to require flags, so I'll look into that instead of doing it this way.
cmd/projects.go
Outdated
func NewProjectsCmd() *cobra.Command { | ||
cmd := &cobra.Command{ | ||
Use: "projects", | ||
Short: "Return a list of projects.", | ||
Long: "Return a list of projects.", | ||
RunE: runProjectsGet, | ||
} | ||
|
||
return cmd | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it looks like we're not defining the list
action anywhere? or are we going with the default of omitting the action means a GET equivalent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops, I forgot to add that. Updated.
"context" | ||
"errors" | ||
"fmt" | ||
"ld-cli/internal/projects" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit but do we care about imports ordering?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fixed this in another PR.
`ldcli projects list` returns list of projects in JSON
Creates
ldcli projects list
command that returns the list of projects as JSON.