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
Merged

Consistent object naming in usage command and examples #704

merged 1 commit into from
Oct 24, 2019

Conversation

carolinebriaud
Copy link
Contributor

@carolinebriaud carolinebriaud commented Oct 21, 2019 •
edited
Loading

Signed-off-by: Caroline Briaud [email protected]

- 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 •
edited
Loading

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" [email protected]: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.

eunomie
eunomie reviewed Oct 23, 2019
View reviewed changes
Copy link
Contributor

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

@carolinebriaud carolinebriaud force-pushed the fix-usages-examples branch 2 times, most recently from 663eb97 to 25e6834 Compare October 23, 2019 12:44
@silvin-lubecki
Copy link
Contributor

@carolinebriaud can you rebase this PR?

chris-crone
chris-crone reviewed Oct 24, 2019
View reviewed changes
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

chris-crone
chris-crone approved these changes Oct 24, 2019
View reviewed changes
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

@carolinebriaud
[b9bd12a] Fix object naming in command help and update examples, e2E …
b07e744 
…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 <[email protected]>
silvin-lubecki
silvin-lubecki approved these changes Oct 24, 2019
View reviewed changes
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.
Reviewers

@ndeloof ndeloof ndeloof approved these changes

@eunomie eunomie eunomie left review comments

@chris-crone chris-crone chris-crone approved these changes

@silvin-lubecki silvin-lubecki silvin-lubecki approved these changes

Assignees
No one assigned
Labels
status/0-triage
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@carolinebriaud @GordonTheTurtle @silvin-lubecki @ndeloof @eunomie @chris-crone