linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Goffredo Baroncelli <kreijack@inwind.it>
To: unlisted-recipients:; (no To-header on input)
Cc: Hugo Mills <hugo@carfax.org.uk>,
	Chris Mason <chris.mason@fusionio.com>,
	"linux-btrfs@vger.kernel.org" <linux-btrfs@vger.kernel.org>
Subject: Re: [PATCH][BTRFS-PROGS][V1] btrfs filesystem df
Date: Wed, 03 Oct 2012 18:17:53 +0200	[thread overview]
Message-ID: <506C6531.4060700@inwind.it> (raw)
In-Reply-To: <20121003115615.GB25498@carfax.org.uk>

On 10/03/2012 01:56 PM, Hugo Mills wrote:
>     Looks good. Only a few comments, inline.
>
> On Wed, Oct 03, 2012 at 01:43:14PM +0200, Goffredo Baroncelli wrote:
>> $ ./btrfs filesystem df --help
>> usage: btrfs filesystem disk-usage [-d][-s][-k]<path>  [<path>..]
>>
>>      Show space usage information for a mount point(s).
>>
>>      -k  Set KB (1024 bytes) as unit
>>      -s  Don't show the summary section
>>      -d  Don't show the detail section
>
>     These are kind of logical, but I think would be hard to remember
> the right way round. I would suggest swapping the actions of the
> switches, and rewording the help:
>
> -s Show only summary section
> -d Show only detail section

Yes this make sense...

>
>> $ ./btrfs filesystem df /
>> Path: /
>> Summary:
>>    Disk_size:                 72.57GB
>
>                                      ^ space between the value and the
>                                        unit (as ISO says), throughout.
>                                        This also makes it easier to
>                                        parse, if anyone wants to.
>
>     Also, use kB, MB, GB, TB for powers-of-ten based units, and KiB,
> MiB, GiB, TiB for powers-of-two based units, please. I don't care
> which you report in, but please do make the distinction. (And note
> that it's kB with a lower case k, but KiB with an upper case K). This
> brings us in line with the relevant ISO and IEEE standards.

I forgot to reply you when you raised this question the first time. Even 
though I am inclined to accept your suggestions, this change is not 
related to my patches. My code uses the functions print_sizes(), which 
is quite old (about 2008). This function is used in a lot of places. 
This suggested to address this issue with another patch.

>
>>    Disk_allocated:            25.10GB
>>    Disk_unallocated:          47.48GB
>>    Logical_size:              23.06GB
>>    Used:                      11.01GB
>>    Free_(Estimated):          55.66GB    (Max: 59.52GB, Min: 35.78GB)
>>    Data_to_disk_ratio:           92 %
>>
>> Details:
>>    Chunk-type  Mode      Chunk-size Logical-size        Used
>>    Data        Single       21.01GB      21.01GB     10.34GB
>>    System      DUP          80.00MB      40.00MB      4.00KB
>>    System      Single        4.00MB       4.00MB        0.00
>>    Metadata    DUP           4.00GB       2.00GB    686.93MB
>>    Metadata    Single        8.00MB       8.00MB        0.00
>
>     Why are the field headings here using - where the field headings in
> the first section used _? Should you be using _ in both places?

2 persons highlighted that :-( ... I will update the code

>
>     Hugo.
>


  reply	other threads:[~2012-10-03 16:18 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-03 11:43 [PATCH][BTRFS-PROGS][V1] btrfs filesystem df Goffredo Baroncelli
2012-10-03 11:43 ` [PATCH 1/2] Update btrfs filesystem df command Goffredo Baroncelli
2012-10-03 15:02   ` Ilya Dryomov
2012-10-03 16:34     ` Goffredo Baroncelli
2012-10-03 17:20       ` Ilya Dryomov
2012-10-03 17:38         ` Goffredo Baroncelli
2012-10-03 17:09     ` Goffredo Baroncelli
2012-10-03 11:43 ` [PATCH 2/2] Update help page Goffredo Baroncelli
2012-10-03 11:56 ` [PATCH][BTRFS-PROGS][V1] btrfs filesystem df Hugo Mills
2012-10-03 16:17   ` Goffredo Baroncelli [this message]
2012-10-03 16:34     ` Hugo Mills
2012-10-09  9:43   ` Bart Noordervliet
2012-10-09 11:38     ` Goffredo Baroncelli
2012-10-09 12:51       ` Bart Noordervliet
2012-10-09 18:22         ` Goffredo Baroncelli
2012-10-12  9:42           ` Martin Steigerwald
2012-10-03 15:01 ` Ilya Dryomov
2012-10-03 16:46   ` Goffredo Baroncelli
2012-10-03 17:46     ` Ilya Dryomov
2012-10-03 20:01       ` Goffredo Baroncelli
2012-10-03 20:24         ` Ilya Dryomov
2012-10-12 10:01       ` Martin Steigerwald
2012-10-12  9:55   ` Martin Steigerwald

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=506C6531.4060700@inwind.it \
    --to=kreijack@inwind.it \
    --cc=chris.mason@fusionio.com \
    --cc=hugo@carfax.org.uk \
    --cc=linux-btrfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).