GNOME Bugzilla – Bug 744603
Show disk usage
Last modified: 2016-03-31 13:22:07 UTC
Created attachment 296950 [details] [review] Add used info for volume devices Instead of only show the disk size, also show its usage. See the attached patch.
Review of attachment 296950 [details] [review]: * No need to put "used" in quotes, lets use the term allocation here too since this is intended more for devs * 'devices' is redundant. Instead i'd call them 'storage volume'. * 'of show' -> 'of showing'. * Some justification for this change would be nice to have. How about "Its not very clear to user what 'Maximum disk' means so lets also show allocation to help with that"? ::: src/i-properties-provider.vala @@ +91,3 @@ uint64 min, uint64 max, + uint64 used, Since its called allocation by libvirt, we should call it the same in the code for consistency @@ +98,3 @@ + label.set_text (format_size (size,format_flags)); + else { + var markup_text = _("%s <span color=\"grey\">(%s used)</span>").printf (format_size (size, format_flags), same comment as for other variable of the same name. ::: src/libvirt-machine-properties.vala @@ +510,3 @@ if (min_storage >= max_storage) { + var label = new Gtk.Label (""); + var markup_text = _("<span color=\"grey\">Maximum Disk Size</span>\t\t %s <span color=\"grey\">(%s free)</span>").printf (format_size (volume_info.capacity, not a good idea to name variables to describe their type. Just text or markup would be fine. @@ +513,3 @@ + FormatSizeFlags.DEFAULT), + format_size (volume_info.allocation, + FormatSizeFlags.DEFAULT)); two redundant empty lines
Review of attachment 296950 [details] [review]: ::: src/libvirt-machine-properties.vala @@ +513,3 @@ + FormatSizeFlags.DEFAULT), + format_size (volume_info.allocation, + FormatSizeFlags.DEFAULT)); Not empty lines. Just really long lines split in order to be smaller. But looks like I didn't achieve the point, :-)
Created attachment 297024 [details] [review] Add allocation info for storage volume
Review of attachment 297024 [details] [review]: Almost there! * 'Add allocation info for' -> 'Show allocation of' ::: src/i-properties-provider.vala @@ +99,3 @@ + else { + var markup = _("%s <span color=\"grey\">(%s used)</span>").printf (format_size (size, format_flags), + format_size (allocation, format_flags)); would be nice to divide this long lines using variables. @@ +129,3 @@ + else { + var markup = _("%s <span color=\"grey\">(%s used)</span>").printf (format_size (v, format_flags), + format_size (allocation, format_flags)); same here ::: src/libvirt-machine-properties.vala @@ +510,3 @@ if (min_storage >= max_storage) { + var label = new Gtk.Label (""); + var markup_text = _("<span color=\"grey\">Maximum Disk Size</span>\t\t %s <span color=\"grey\">(%s used)</span>").printf (format_size (volume_info.capacity, FormatSizeFlags.DEFAULT), * still called markup_text. * Same thing about dividing this a bit using variables.
Created attachment 297062 [details] [review] Show allocation of storage volume Instead of creating and returning a Gtk.Label, as suggested, the set_label_msg() function is taking a label as argument, as we don't want to create new labels every time the scale value is changed.
Review of attachment 297062 [details] [review]: Please push with this fixed. ::: src/i-properties-provider.vala @@ +75,3 @@ private FormatSizeFlags format_flags; + private static void set_label_msg (Gtk.Label label, * same issue as with set_markup. what label and msg? How about "set_size_label". * argument names need to be aligned too. @@ +77,3 @@ + private static void set_label_msg (Gtk.Label label, + uint64 size, + uint64 alloc, 'allocation' isn't big enough to justify shortening.
Review of attachment 297062 [details] [review]: ::: src/i-properties-provider.vala @@ +77,3 @@ + private static void set_label_msg (Gtk.Label label, + uint64 size, + uint64 alloc, A few lines below you will see that I'm using "allocation" as another variable name. Thus my preference was to use the shorter form as the attribute name, as we already do it for "maximum" and "minimum". Is it okay for you?
Review of attachment 297062 [details] [review]: ::: src/i-properties-provider.vala @@ +77,3 @@ + private static void set_label_msg (Gtk.Label label, + uint64 size, + uint64 alloc, that one is a string for just name that "allocation_str" or "allocation_markup" or just "markup" since its used only once.
Created attachment 297068 [details] [review] Show allocation of storage volume Pushed as 1e99268223a096ded826beee5142a4a9f782fcca (3.16+)