1070002 - Remove all filesize declarations to be automatically managed

Status: RESOLVED FIXED

(www.mozilla.org :: Product Details, defect)

Description

[Sylvestre Ledru [:Sylvestre]]

Attachment: filesize-managed.diff

Product details has plenty of hardcoded filesize information. Most of them are set to 0 and nobody cares about them (at least for fennec and firefox, thunderbird relman are still updating them).

The attached proposes to remove the declarations and automatically fill the data to 0.

Next step will be to remove the values from the json file but that is a different story^Wbug

Comment 1

[Sylvestre Ledru [:Sylvestre]]

Attachment: json-export.diff

Just for information, here is the json file generated with the changes (oh, I <3 the pretty print)

Comment 2

[Paul [:pmac] McLanahan]

Looks good to me. I'm almost positive that the filesize info has been ignored for a while as very few people care, and it's been mostly out of date for a long time.

Comment 3

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8492236 [details] [diff] [review]
filesize-managed.diff

Looks good to me. Although I think this change is unlikely to break anything, I'm relying on others to confirm that this won't break things.

Comment 4

[Pascal Chevrel:pascalc]

Comment on attachment 8492236 [details] [diff] [review]
filesize-managed.diff

Review of attachment 8492236 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with nits

::: export_json.php
@@ +41,5 @@
> +
> +/* Create the filesize data structure which is going to be
> + * duplicated
> +*/
> +    $OS = array("Windows", "OS X", "Linux");

Nits: Since you use this $OS variable only once in the foreach, you could pass it directly to the foreach loop and avoid creating a global variable. Also, short array syntax is easier to read and double quotes are to use when you intend to expand variales, ex:
foreach (['Windows', 'OS X', 'Linux'] as $os)

@@ +46,5 @@
> +    $filesize = array();
> +    foreach ($OS as $o) {
> +        $filesize[$o] = array("filesize" => 0);
> +
> +    }

I am not sure the code above is clearer than just defining an array:
$filesize = [
    'Windows' => ['filesize' => 0],
    'OS X'    => ['filesize' => 0],
    'Linux'   => ['filesize' => 0],
];

@@ +66,5 @@
>  $fxd = new firefoxDetails();
>  $builds = array('primary_builds', 'beta_builds');
>  foreach ($builds as $build) {
> +    $buildArray = fillFileSize($fxd->$build);
> +    writefile("firefox_{$build}.json", $buildArray);

Not related to your patch itself, but the function writefile() should be writeJsonFile() I think :)

Comment 5

[Sylvestre Ledru [:Sylvestre]]

Right, I over designed it. Thanks for the feedback.

Merged:
http://viewvc.svn.mozilla.org/vc?view=revision&revision=131977
And regeneration:
http://viewvc.svn.mozilla.org/vc?view=revision&revision=131978