Skip to content

Conversation

@friism
Copy link
Contributor

@friism friism commented Jan 4, 2018

- What I did

The help text for config commands is not consistent about whether the object being modified is called a "config" or a "configuration file". I changed it to be "config" for all commands since that's shorter.

Previously:

Commands:
  create      Create a configuration file from a file or STDIN as content
  inspect     Display detailed information on one or more configuration files
  ls          List configs
  rm          Remove one or more configuration files

- How to verify it

docker config --help

- Description for the changelog

Descriptions for config commands now use the same name when describing configs.

@codecov-io
Copy link

codecov-io commented Jan 4, 2018

Codecov Report

Merging #785 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff           @@
##           master    #785   +/-   ##
======================================
  Coverage    50.9%   50.9%           
======================================
  Files         237     237           
  Lines       15338   15338           
======================================
  Hits         7808    7808           
  Misses       7028    7028           
  Partials      502     502

Copy link
Contributor

@dnephin dnephin left a comment

Choose a reason for hiding this comment

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

SGTM

one nit on wording for create

cmd := &cobra.Command{
Use: "create [OPTIONS] CONFIG file|-",
Short: "Create a configuration file from a file or STDIN as content",
Short: "Create config using a file or STDIN as content",
Copy link
Contributor

Choose a reason for hiding this comment

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

How about:

"Create a config from a file or STDIN"

Copy link
Contributor

@dnephin dnephin left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

LGTM 😛
cc @thaJeztah

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM "configuration file" was in an attempt to make the purpose more clear, but I agree it wasn't consistent, and not really adding much

@friism can you squash your commits? Then we can merge 👍

Signed-off-by: Michael Friis <friism@gmail.com>
@friism friism force-pushed the make-config-wording-more-consistent branch from fc8eb74 to 99e3b4c Compare January 5, 2018 21:20
@friism
Copy link
Contributor Author

friism commented Jan 5, 2018

squashed, thanks guys

@thaJeztah thaJeztah merged commit a60bdf3 into docker:master Jan 5, 2018
@GordonTheTurtle GordonTheTurtle added this to the 18.01.0 milestone Jan 5, 2018
@thaJeztah thaJeztah modified the milestones: 18.01.0, 18.02.0 Jan 9, 2018
nobiit pushed a commit to nobidev/docker-cli that referenced this pull request Nov 19, 2025
…nsistent

use 'config' over 'configuration file'
Upstream-commit: a60bdf3
Component: cli
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants