Skip to content

More linters and better circle-ci config#27

Merged
dnephin merged 4 commits into
docker:masterfrom
dnephin:more-linters
May 8, 2017
Merged

More linters and better circle-ci config#27
dnephin merged 4 commits into
docker:masterfrom
dnephin:more-linters

Conversation

@dnephin

@dnephin dnephin commented May 3, 2017

Copy link
Copy Markdown
Contributor

Add the last 2 linters.

lll checks line length. 200 is rather long, but it's a start
interfacer checks for when an unnecessarily large interface is used.

I've also updated the circleci config to run each of the steps in parallel. This cuts down the build time to just the slowest step, which is currently cross. With #26 cross is going to get even slower.

With this change we can also download binaries for each build: https://circleci.com/gh/docker/cli/51#artifacts/containers/1

@dnephin dnephin force-pushed the more-linters branch 4 times, most recently from 8ff879a to a004407 Compare May 4, 2017 00:57
@dnephin dnephin changed the title More linters More linters and better circle-ci config May 4, 2017
@dnephin dnephin force-pushed the more-linters branch 2 times, most recently from 30922c6 to cb86678 Compare May 4, 2017 01:25

@vdemeester vdemeester left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Overall LGTM 👼
We might want to wait for #11 to get merged to have less conflicts ?


// NewContainerCommand returns a cobra command for `container` subcommands
// nolint: interfacer
func NewContainerCommand(dockerCli *command.DockerCli) *cobra.Command {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@dnephin why not this one for command.Cli ?

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.

There are some auth methods that don't exist on the interface yet. And if I change this I have to change everything in the container/ package, so I wanted to delay that change.

Most // nolint should be considered the equivalent of a // TODO: fix this

// PushTrustedReference pushes a canonical reference to the trust server.
// nolint: gocyclo
func PushTrustedReference(cli *command.DockerCli, repoInfo *registry.RepositoryInfo, ref reference.Named, authConfig types.AuthConfig, in io.Reader) error {
func PushTrustedReference(streams command.Streams, repoInfo *registry.RepositoryInfo, ref reference.Named, authConfig types.AuthConfig, in io.Reader) error {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

😻


// NewNetworkCommand returns a cobra command for `network` subcommands
// nolint: interfacer
func NewNetworkCommand(dockerCli *command.DockerCli) *cobra.Command {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Same here.

Comment thread cli/command/plugin/cmd.go

// NewPluginCommand returns a cobra command for `plugin` subcommands
// nolint: interfacer
func NewPluginCommand(dockerCli *command.DockerCli) *cobra.Command {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

and here 👼

Comment thread cli/command/secret/cmd.go

// NewSecretCommand returns a cobra command for `secret` subcommands
// nolint: interfacer
func NewSecretCommand(dockerCli *command.DockerCli) *cobra.Command {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

here too, and below 😛

@dnephin

dnephin commented May 4, 2017

Copy link
Copy Markdown
Contributor Author

Sure, I'm fine to wait

@gdevillele gdevillele 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.

LGTM

Signed-off-by: Daniel Nephin <dnephin@docker.com>
dnephin added 3 commits May 8, 2017 15:57
Signed-off-by: Daniel Nephin <dnephin@docker.com>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
@dnephin dnephin merged commit d844c48 into docker:master May 8, 2017
@dnephin dnephin deleted the more-linters branch May 8, 2017 20:30
@thaJeztah thaJeztah modified the milestone: 17.06.0 Jul 21, 2017
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.

4 participants