Skip to content

chore: Refactor API client and pass into constructor fns - #64

Merged
dbolson merged 3 commits into
mainfrom
refactor-client--create-constructor-fns
Mar 25, 2024
Merged

chore: Refactor API client and pass into constructor fns#64
dbolson merged 3 commits into
mainfrom
refactor-client--create-constructor-fns

Conversation

@dbolson

@dbolson dbolson commented Mar 25, 2024

Copy link
Copy Markdown
Contributor

Refactoring the API client so we can pass it in to the commands. Now we can test the commands from the root without making real API calls.

This is the first of a few PRs to do all that and backfill tests.

Comment thread cmd/flags/create.go
)

func NewCreateCmd() *cobra.Command {
func NewCreateCmd() (*cobra.Command, error) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Constructor fns can return errors, so we bubble those up to the top level.

Comment thread cmd/projects/create.go
)

func NewCreateCmd() *cobra.Command {
func NewCreateCmd(client projects.Client) (*cobra.Command, error) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This passes in the client that's stored in a closure. We need to do this because of how late cobra and viper bind the flags which aren't set yet in the root command. Since we pass in the access token, we don't have the value until we call the subcommand's Run function.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to do this for flags update/create as well? or will that be in a follow up pr

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'll do it in another PR. This was already getting pretty big.

Comment thread cmd/root.go

cmd.PersistentFlags().StringVarP(
&accessToken,
cmd.PersistentFlags().StringP(

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes to a simpler function since we don't need the variable set.

type Client interface {
Create(ctx context.Context, name string, key string) ([]byte, error)
List(ctx context.Context) ([]byte, error)
Create(ctx context.Context, accessToken, baseURI, name, key string) ([]byte, error)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These calls need more info since we can't pass them in until we call these methods.

@dbolson dbolson changed the title Refactor API client and pass into constructor fns chore: Refactor API client and pass into constructor fns Mar 25, 2024

@k3llymariee k3llymariee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a couple of q's to make sure i follow what's going on 😅

Comment thread cmd/projects/create.go
)

func NewCreateCmd() *cobra.Command {
func NewCreateCmd(client projects.Client) (*cobra.Command, error) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to do this for flags update/create as well? or will that be in a follow up pr

Comment thread cmd/root.go
)

func newRootCommand() *cobra.Command {
func NewRootCommand(client projects.Client) (*cobra.Command, error) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

guessing this will also be updated to a more generic client (not specific to projects) in a follow up PR as well?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. Making the changes to the flags commands will force that.

@dbolson
dbolson merged commit 8883281 into main Mar 25, 2024
@dbolson
dbolson deleted the refactor-client--create-constructor-fns branch March 25, 2024 22:08
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