Skip to content

Add format to the docker system df command#31482

Merged
thaJeztah merged 1 commit intomoby:masterfrom
boaz0:add_format_to_system_df
Apr 13, 2017
Merged

Add format to the docker system df command#31482
thaJeztah merged 1 commit intomoby:masterfrom
boaz0:add_format_to_system_df

Conversation

@boaz0
Copy link
Contributor

@boaz0 boaz0 commented Mar 2, 2017

- What I did
Add the format option to the docker-system-df command
related to #30431

- How I did it

  • Add the format option to the cli/command/system/df.go
  • Add the NewDiskUsageFormat function to return a right format from options in cli/command/formatter/disk_usage.go
  • Add unit tests

- How to verify it
TESTDIRS='cli/command/formatter/' TESTFLAGS='-test.run ^TestDiskUsage*' hack/make.sh test-unit

- Description for the changelog

  • Add format to docker-system-df

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

todo: add to docs

Copy link
Member

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

Design SGTM
/cc @thaJeztah @dnephin @mlaventure

Copy link
Member

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

Design LGTM

Copy link
Member

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

@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 for docs-review
@ripcurld0 You will need to updates docs I think 👼

@boaz0
Copy link
Contributor Author

boaz0 commented Apr 3, 2017

Yes I will.
Will do it tonight 😴

@boaz0
Copy link
Contributor Author

boaz0 commented Apr 3, 2017

DONE 😀 🎸 @thaJeztah

Signed-off-by: Boaz Shuster <ripcurld.github@gmail.com>
<Paste>
```

**Note** the format option is meaningless when verbose is true.
Copy link
Member

Choose a reason for hiding this comment

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

Should we error out this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, we can. I will do that soon.

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps;

> **Note**: the format option is ignored if `--verbose` is used

However, since you're planning to do a follow up to error out, let's keep it for now (you can remove it in that PR)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

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.

docs LGTM, thanks!

@thaJeztah thaJeztah merged commit 0b35ab1 into moby:master Apr 13, 2017
@thaJeztah thaJeztah added this to the 17.06 milestone Apr 13, 2017
dnephin pushed a commit to dnephin/docker that referenced this pull request Apr 17, 2017
Add format to the docker system df command
@boaz0 boaz0 deleted the add_format_to_system_df branch October 6, 2017 14:24
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.

7 participants