Skip to content
This repository has been archived by the owner on Jun 13, 2021. It is now read-only.

Consistent object naming in usage command and examples #704

Merged
merged 1 commit into from
Oct 24, 2019

Conversation

carolinebriaud
Copy link
Contributor

@carolinebriaud carolinebriaud commented Oct 21, 2019

Signed-off-by: Caroline Briaud caroline.briaud@docker.com

- What I did
Correct help messages, examples and flag descriptions to match the app naming rules

- How to verify it
Go through help, examples and flag description for each docker app command

- A picture of a cute animal (not mandatory but encouraged)
image

@carolinebriaud carolinebriaud changed the title Fix space issue in docker app image inspect example [WIP] Fix space issue in docker app image inspect example Oct 21, 2019
@carolinebriaud carolinebriaud changed the title [WIP] Fix space issue in docker app image inspect example [WIP] Consistent object naming in usage command and examples Oct 21, 2019
@codecov
Copy link

codecov bot commented Oct 21, 2019

Codecov Report

Merging #704 into master will decrease coverage by 1.01%.
The diff coverage is 95.52%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #704      +/-   ##
=========================================
- Coverage   72.21%   71.2%   -1.02%     
=========================================
  Files          59      59              
  Lines        3146    2997     -149     
=========================================
- Hits         2272    2134     -138     
  Misses        580     580              
+ Partials      294     283      -11
Impacted Files Coverage Δ
internal/commands/pull.go 69.23% <100%> (ø) ⬆️
internal/commands/init.go 86.66% <100%> (ø) ⬆️
internal/commands/image/inspect.go 74.46% <100%> (-4.48%) ⬇️
internal/commands/render.go 70.9% <100%> (ø) ⬆️
internal/commands/image/tag.go 88.88% <100%> (-2.61%) ⬇️
internal/commands/image/list.go 83.33% <100%> (-4.17%) ⬇️
internal/commands/remove.go 52% <100%> (ø) ⬆️
internal/commands/image/rm.go 62.5% <100%> (-1.61%) ⬇️
internal/commands/list.go 84.12% <100%> (ø) ⬆️
internal/commands/root.go 75% <100%> (ø) ⬆️
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e5b6e29...b07e744. Read the comment docs.

@carolinebriaud carolinebriaud changed the title [WIP] Consistent object naming in usage command and examples Consistent object naming in usage command and examples Oct 22, 2019
@carolinebriaud carolinebriaud marked this pull request as ready for review October 22, 2019 16:35
@GordonTheTurtle
Copy link

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "fix-usages-examples" git@github.com:carolinebriaud/app.git somewhere
$ cd somewhere
$ git rebase -i HEAD~842354382296
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

Copy link
Member

@eunomie eunomie left a comment

Choose a reason for hiding this comment

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

I'm not sure about some changes around brackets.
A very common pattern in CLI help is to mark optional fields between brackets and to avoid brackets on all mandatory fields. That way it's easy to know what to provide.

internal/commands/build/build.go Show resolved Hide resolved
internal/commands/build/build.go Outdated Show resolved Hide resolved
internal/commands/image/tag.go Outdated Show resolved Hide resolved
@silvin-lubecki
Copy link
Contributor

@carolinebriaud can you rebase this PR?

Copy link
Member

@chris-crone chris-crone left a comment

Choose a reason for hiding this comment

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

One minor change but then LGTM

Copy link
Member

@chris-crone chris-crone left a comment

Choose a reason for hiding this comment

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

LGTM

…tests and file linting. Remove bracket changes in command usage message. Take into account feedback from Silvin, Chris and Nicolas.

Signed-off-by: Caroline Briaud <caroline.briaud@docker.com>
Copy link
Contributor

@silvin-lubecki silvin-lubecki left a comment

Choose a reason for hiding this comment

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

LGTM

@silvin-lubecki silvin-lubecki merged commit 6e215e7 into docker:master Oct 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants