cli icon indicating copy to clipboard operation
cli copied to clipboard

Design question: what should the cli and api packages actually look like?

Open kytrinyx opened this issue 7 years ago • 4 comments

@nywilken I was looking at the contents of the cli package and the api package. I think we've got a bit of ambiguity around where things belong.

I'm wondering if we should have the CLI-specific logic in the cli package (build stuff, logic around versions, maybe), and then have everything that talks to remote servers in api. But maybe api is the wrong name. Maybe remote?

kytrinyx avatar Aug 26 '18 17:08 kytrinyx

I'm wondering if we should have the CLI-specific logic in the cli package (build stuff, logic around versions, maybe), and then have everything that talks to remote servers in api. But maybe api is the wrong name. Maybe remote?

I think this makes complete sense. Maybe even have a pkg specific for the exercism api. I know I mentioned the upgrade pkg in the past but this might be a good opportunity to revisit.

I’m blanking right now on what is specific to the cli. But it is worth breaking up. I have time this week to put together a rough proposal for the pkg break out.

nywilken avatar Aug 28 '18 01:08 nywilken

I have time this week to put together a rough proposal for the pkg break out.

That would be fantastic. I've been spending an hour or so each evening checking in on discussions, PRs, and issues, so I will have time to look at a proposal if you have one.

kytrinyx avatar Aug 28 '18 01:08 kytrinyx

Current Structure

An outline of the current HTTPClient structure being used by the Exercism CLI

Packages which define an HTTPClient:

github.com/exercism/cli/api
github.com/exercism/cli/cli

Commands which use the defined HTTPClients:

 github.com/exercism/cli/cmd/configure
 github.com/exercism/cli/cmd/download
 github.com/exercism/cli/cmd/submit
 github.com/exercism/cli/cmd/troubleshoot
 github.com/exercism/cli/cmd/version

Import Dependencies:

  github.com/exercism/cli/api
  |_cli/api.go
  |_cmd/configure.go
  |_cmd/download.go
  |_cmd/submit.go

  github.com/exercism/cli/cli
  |_cmd/troubleshoot.go
  |_cmd/upgrade.go
  |_cmd/version.go

Proposed Structure

cmd/... (command files; minor changes)
api/ (Exercism configured API client; we can restructure further)
|_api.go
|_debug.go
pkg/ (Re-usable packages)
|_cli/ (great candidate for the GitHub Release Updater pkg; needs to be renamed)
   |_ cli.go
   |_ asset.go
   |_ release.go

In addition to the new structure, I propose that we add a third HTTPClient. The third client would live in cmd/root.go for now and it would be configured specifically for the use of the Exercism CLI; more specifically anything that is not called through the API client or the new github updating pkg.

With the addition of the third HTTPClient (lets call it cmd.httpClient for now) we can tune that client for the application (custom timeout), which can then be injected into the APIClient, CLI Updater pkg, and any other HTTP service we create. This way all HTTP services use the one tuned client. Allowing us to enforce the same configuration on all HTTP calls, and providing a way to make HTTP calls without having to rely on api.HTTPClient or, the currently name, cli.HTTPClient both of which are configured for specific endpoints.

With the advent of the third client we could then keep the api.Client calls separate from other HTTP calls (e.g cmd.httpClient.Get(...))

To help provide an idea of what this proposal looks like I opened up a WIP PR.

nywilken avatar Aug 31 '18 02:08 nywilken

With the addition of the third HTTPClient (lets call it cmd.httpClient for now) we can tune that client for the application (custom timeout), which can then be injected into the APIClient, CLI Updater pkg, and any other HTTP service we create.

I like this.

I'll take a look at that PR!

kytrinyx avatar Sep 03 '18 17:09 kytrinyx