Merge lp:~diegosarmentero/click-update-manager/bytes-to-size into lp:click-update-manager

[Diego Sarmentero]

- Show app size in the proper way (BUG: #1232256).

[lp:~diegosarmentero/click-update-manager/bytes-to-size]

37. By Diego Sarmentero on 2013-10-09

fix if comparison

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:37
http://jenkins.qa.ubuntu.com/job/click-update-manager-ci/35/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/click-update-manager-saucy-amd64-ci/35/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/click-update-manager-saucy-armhf-ci/35/console

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/click-update-manager-ci/35/rebuild

review: Needs Fixing (continuous-integration)

[Mike McCracken (mikemc)]

In utils.js, the calculation "parseFloat(bytes / 1024).toFixed(2)" is strange.
per IRC, bytes will be an int, so we're dividing two ints then apparently implicitly converting them to strings before passing them to parseFloat(), which generates a float.

If you just remove "parseFloat" and divide by a float, it works the same but makes more sense to me.

like this:

var size = (bytes / 1024.0).toFixed(2);

review: Needs Fixing

[Mike McCracken (mikemc)]

http://paste.ubuntu.com/6214313/ is the QML I was using to poke at the convert_bytes_to_size function

[lp:~diegosarmentero/click-update-manager/bytes-to-size]

38. By Diego Sarmentero on 2013-10-11

removing parseFloat

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:38
http://jenkins.qa.ubuntu.com/job/click-update-manager-ci/36/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/click-update-manager-saucy-amd64-ci/36/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/click-update-manager-saucy-armhf-ci/36/console

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/click-update-manager-ci/36/rebuild

review: Needs Fixing (continuous-integration)

[lp:~diegosarmentero/click-update-manager/bytes-to-size]

39. By Diego Sarmentero on 2013-10-14

update cmake

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:39
http://jenkins.qa.ubuntu.com/job/click-update-manager-ci/37/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/click-update-manager-saucy-amd64-ci/37/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/click-update-manager-saucy-armhf-ci/37/console

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/click-update-manager-ci/37/rebuild

review: Needs Fixing (continuous-integration)

[lp:~diegosarmentero/click-update-manager/bytes-to-size]

40. By Diego Sarmentero on 2013-10-15

revert desktop file name

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:40
http://jenkins.qa.ubuntu.com/job/click-update-manager-ci/38/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/click-update-manager-saucy-amd64-ci/38/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/click-update-manager-saucy-armhf-ci/38/console

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/click-update-manager-ci/38/rebuild

review: Needs Fixing (continuous-integration)

[lp:~diegosarmentero/click-update-manager/bytes-to-size]

41. By Diego Sarmentero on 2013-10-16

adding desktop file

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:41
http://jenkins.qa.ubuntu.com/job/click-update-manager-ci/39/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/click-update-manager-saucy-amd64-ci/39
    SUCCESS: http://jenkins.qa.ubuntu.com/job/click-update-manager-saucy-armhf-ci/39

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/click-update-manager-ci/39/rebuild

review: Approve (continuous-integration)

[Roberto Alsina (ralsina)]

Asked for GiB to be added just in case.

review: Approve

[lp:~diegosarmentero/click-update-manager/bytes-to-size]

42. By Diego Sarmentero on 2013-10-16

adding GiB to convert function

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:42
http://jenkins.qa.ubuntu.com/job/click-update-manager-ci/40/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/click-update-manager-saucy-amd64-ci/40
    SUCCESS: http://jenkins.qa.ubuntu.com/job/click-update-manager-saucy-armhf-ci/40

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/click-update-manager-ci/40/rebuild

review: Approve (continuous-integration)

[dobey (dobey)]

> === added file 'click-update-manager.desktop'
> === removed file 'click-update-manager.desktop'

Huh? How can it be both added and removed in the same diff? And which one is correct?

82 +function convert_bytes_to_size(bytes) {
83 + var result = "";
84 + var size = (bytes / 1024.0).toFixed(2);
85 + if(size < 1024) {
86 + result = size + " KiB";
87 + } else if(size < 1048576) {
88 + size = (size / 1024.0).toFixed(2);
89 + result = size + " MiB";
90 + } else {
91 + size = (size / 1048576.0).toFixed(2);
92 + result = size + " GiB";
93 + }
94 +
95 + return result;
96 +}

This code is really hard to read, because the checks are inconsistent, and you're converting bytes to KiB from the start, but use "size" as a variable name. Why not just use the input "bytes" variable across the board for checking, and only place the reduced size in the result string? Something like this perhaps:

var SIZE_IN_GIB = 1024.0 * 1024.0 * 1024.0;
var SIZE_IN_MIB = 1024.0 * 1024.0;
var SIZE_IN_KIB = 1024.0;

var result = "";
var size = 0;
if (bytes / SIZE_IN_KIB < SIZE_IN_KIB) {
    size = bytes / SIZE_IN_KIB;
    result = size + " KiB";
} else if (bytes / SIZE_IN_MIB < SIZE_IN_KIB) {
    size = bytes / SIZE_IN_MIB;
    result = size + " MiB";
} else if (bytes / SIZE_IN_GIB < SIZE_IN_KIB) {
    size = bytes / SIZE_IN_GIB;
    result = size + " GiB";
} else {
    result = bytes + " bytes";
}

return result;

review: Needs Fixing

[lp:~diegosarmentero/click-update-manager/bytes-to-size]

43. By Diego Sarmentero on 2013-10-16

improve convert function

[Diego Sarmentero (diegosarmentero)]

> > === added file 'click-update-manager.desktop'
> > === removed file 'click-update-manager.desktop'
>
> Huh? How can it be both added and removed in the same diff? And which one is
> correct?
>
> 82 +function convert_bytes_to_size(bytes) {
> 83 + var result = "";
> 84 + var size = (bytes / 1024.0).toFixed(2);
> 85 + if(size < 1024) {
> 86 + result = size + " KiB";
> 87 + } else if(size < 1048576) {
> 88 + size = (size / 1024.0).toFixed(2);
> 89 + result = size + " MiB";
> 90 + } else {
> 91 + size = (size / 1048576.0).toFixed(2);
> 92 + result = size + " GiB";
> 93 + }
> 94 +
> 95 + return result;
> 96 +}
>
> This code is really hard to read, because the checks are inconsistent, and
> you're converting bytes to KiB from the start, but use "size" as a variable
> name. Why not just use the input "bytes" variable across the board for
> checking, and only place the reduced size in the result string? Something like
> this perhaps:
>
> var SIZE_IN_GIB = 1024.0 * 1024.0 * 1024.0;
> var SIZE_IN_MIB = 1024.0 * 1024.0;
> var SIZE_IN_KIB = 1024.0;
>
> var result = "";
> var size = 0;
> if (bytes / SIZE_IN_KIB < SIZE_IN_KIB) {
> size = bytes / SIZE_IN_KIB;
> result = size + " KiB";
> } else if (bytes / SIZE_IN_MIB < SIZE_IN_KIB) {
> size = bytes / SIZE_IN_MIB;
> result = size + " MiB";
> } else if (bytes / SIZE_IN_GIB < SIZE_IN_KIB) {
> size = bytes / SIZE_IN_GIB;
> result = size + " GiB";
> } else {
> result = bytes + " bytes";
> }
>
> return result;

Fixed

[lp:~diegosarmentero/click-update-manager/bytes-to-size]

44. By Diego Sarmentero on 2013-10-16

revert renaming desktop file

[dobey (dobey)]

review: Approve

[Mike McCracken (mikemc)]

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:43
http://jenkins.qa.ubuntu.com/job/click-update-manager-ci/41/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/click-update-manager-saucy-amd64-ci/41
    SUCCESS: http://jenkins.qa.ubuntu.com/job/click-update-manager-saucy-armhf-ci/41

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/click-update-manager-ci/41/rebuild

review: Approve (continuous-integration)

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2013-10-03 02:35:10 +0000
+++ CMakeLists.txt	2013-10-16 18:03:12 +0000
@@ -8,6 +8,7 @@
 file(GLOB QML_JS_FILES *.qml)
 install(FILES ${QML_JS_FILES} DESTINATION ${CLICK_UPDATE_MANAGER_DIR})
 install(DIRECTORY Components DESTINATION ${CLICK_UPDATE_MANAGER_DIR})
+install(DIRECTORY js DESTINATION ${CLICK_UPDATE_MANAGER_DIR})
 
 install(FILES click-update-manager.desktop DESTINATION ${CMAKE_INSTALL_DATADIR}/applications)
 
 
=== modified file 'Components/PageUpdate.qml'
--- Components/PageUpdate.qml	2013-10-08 13:15:38 +0000
+++ Components/PageUpdate.qml	2013-10-16 18:03:12 +0000
@@ -18,6 +18,7 @@
 import Ubuntu.Components 0.1
 import Ubuntu.Components.ListItems 0.1 as ListItem
 import com.ubuntu.click 0.1
+import "../js/utils.js" as Utils
 
 Page {
     id: root
@@ -160,7 +161,7 @@
                         }
                     }
                     Label {
-                        text: parseFloat((modelData.binaryFilesize / 1024) / 1024).toFixed(2) + " Mb"
+                        text: Utils.convert_bytes_to_size(modelData.binaryFilesize)
                         height: textArea.height / 2
                         anchors.right: parent.right
                     }
 
=== added directory 'js'
=== added file 'js/utils.js'
--- js/utils.js	1970-01-01 00:00:00 +0000
+++ js/utils.js	2013-10-16 18:03:12 +0000
@@ -0,0 +1,39 @@
+/*
+ * Copyright (C) 2013 Canonical, Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+.pragma library
+
+function convert_bytes_to_size(bytes) {
+    var SIZE_IN_GIB = 1024.0 * 1024.0 * 1024.0;
+    var SIZE_IN_MIB = 1024.0 * 1024.0;
+    var SIZE_IN_KIB = 1024.0;
+
+    var result = "";
+    var size = 0;
+    if (bytes / SIZE_IN_KIB < SIZE_IN_KIB) {
+        size = (bytes / SIZE_IN_KIB).toFixed(2);
+        result = size + " KiB";
+    } else if (bytes / SIZE_IN_MIB < SIZE_IN_KIB) {
+        size = (bytes / SIZE_IN_MIB).toFixed(2);
+        result = size + " MiB";
+    } else if (bytes / SIZE_IN_GIB < SIZE_IN_KIB) {
+        size = (bytes / SIZE_IN_GIB).toFixed(2);
+        result = size + " GiB";
+    } else {
+        result = bytes + " bytes";
+    }
+
+    return result;
+}
 
=== modified file 'tests/unit/tst_pageupdate.qml'
--- tests/unit/tst_pageupdate.qml	2013-10-08 13:15:38 +0000
+++ tests/unit/tst_pageupdate.qml	2013-10-16 18:03:12 +0000
@@ -154,7 +154,7 @@
                 compare(colLeft.children[1].visible, true, "Should be visible");
                 compare(colLeft.children[1].text, "Version: " + fakeModel[i].remoteVersion, "Remote versions don't match");
                 var colRight = UT.findChild(obj, "colRight");
-                var mbvalue = parseFloat((fakeModel[i].binaryFilesize / 1024) / 1024).toFixed(2) + " Mb";
+                var mbvalue = "4.88 KiB";
                 compare(colRight.children[1].text, mbvalue, "Other Mb size expected");
             }
         }
 
=== modified file 'updatemanager.qmlproject'
--- updatemanager.qmlproject	2013-09-20 20:04:44 +0000
+++ updatemanager.qmlproject	2013-10-16 18:03:12 +0000
@@ -26,6 +26,10 @@
         filter: "*"
     }
     Files {
+        directory: "js/"
+        filter: "*"
+    }
+    Files {
         directory: "img/"
         filter: "*"
     }