Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: ipfs repo stat #2430

Merged
merged 4 commits into from
Mar 11, 2016
Merged

Feature: ipfs repo stat #2430

merged 4 commits into from
Mar 11, 2016

Conversation

daviddias
Copy link
Member

Implements #1744
Builds on top of #2289

> ipfs repo stat
NumObjects      11
RepoSize        52064
RepoPath        /Users/david/.ipfs

License: MIT
Signed-off-by: Thomas Gardner <tmg@fastmail.com>
License: MIT
Signed-off-by: Thomas Gardner <tmg@fastmail.com>
License: MIT
Signed-off-by: Thomas Gardner <tmg@fastmail.com>
fmt.Fprintf(buf, "RepoSize \t %d\n", stat.RepoSize)
fmt.Fprintf(buf, "RepoPath \t %s\n", stat.RepoPath)

return strings.NewReader(buf.String()), nil
Copy link
Member

Choose a reason for hiding this comment

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

you can just do:

return buf, nil

here, all it wants is an io.Reader which the buffer is

@whyrusleeping
Copy link
Member

Mind venturing into the wild world of sharness tests? Would be nice to have some coverage on this.

@daviddias daviddias force-pushed the feat/repo-stat branch 4 times, most recently from 44594f0 to 20f43fb Compare March 2, 2016 17:41
@daviddias
Copy link
Member Author

@whyrusleeping added :)

test_expect_success "'ipfs repo stat' succeeds" '
ipfs repo stat > repo-stats &&
grep "RepoPath" repo-stats &&
grep "RepoSize" repo-stats &&
Copy link
Member

Choose a reason for hiding this comment

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

I would break this up into more fine grained tests, makes debugging easier later on, maybe do:

  • ipfs repo stat
  • check that it looks right
  • add the file and rerun repo stat
  • check that the second stat looks right

where each item there is a separate test_expect_success

'ipfs repo stat' is a plumbing command that will scan the local
set of stored objects and print repo statistics. It outputs to stdout:
NumObjects int number of objects in the local repo
RepoSize int size that the repo is currently taking
Copy link
Member

Choose a reason for hiding this comment

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

Size in bits? ;)

@RichardLitt
Copy link
Member

Watching.

@Kubuxu
Copy link
Member

Kubuxu commented Mar 3, 2016

-h flag for human readable units would be nice (if yes then don't forget about MB vs MiB).

@daviddias daviddias force-pushed the feat/repo-stat branch 2 times, most recently from ded0e0c to 5738c4c Compare March 4, 2016 17:58
@daviddias
Copy link
Member Author

@Kubuxu added as --human, since we can't use -h as it is used for the help menu

@whyrusleeping
Copy link
Member

LGTM, thanks!

buf := new(bytes.Buffer)
fmt.Fprintf(buf, "NumObjects \t %d\n", stat.NumObjects)
if human {
fmt.Fprintf(buf, "RepoSize (MiB) \t %d\n", stat.RepoSize/1024)
Copy link

Choose a reason for hiding this comment

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

If stat.RepoSize is bytes as line 106 suggests, you need to devide by 1024^2 here

Copy link

Choose a reason for hiding this comment

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

Maybe also worth taking sub-MiB sizes into account here -- this might turn out as 0 if it's less than a MiB?

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks for catching that @lgierth :)

check #c10e329, I've added a clause to print it in bytes if it is less than one MiB. What do you think?

License: MIT
Signed-off-by: David Dias <daviddias.p@gmail.com>
@ghost
Copy link

ghost commented Mar 5, 2016

LGTM 👍 :)

@atomgardner
Copy link
Contributor

Many thanks for the clean up. You might add the [Bash complete] too.

[Bash complete] https://github.com/tomgg/go-ipfs/commit/9643b94d7ca9027138ea65e78f7875ad6be259aa

@whyrusleeping
Copy link
Member

I'm going to merge this as is, @tomgg if you want to PR in the bash completion please do ( i figure it would take you less time that anyone else since youve done it before)

whyrusleeping added a commit that referenced this pull request Mar 11, 2016
@whyrusleeping whyrusleeping merged commit 286a596 into ipfs:master Mar 11, 2016
@daviddias daviddias deleted the feat/repo-stat branch March 11, 2016 21:14
@daviddias
Copy link
Member Author

🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉

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.

6 participants