Remove all filesize declarations to be automatically managed

RESOLVED FIXED

Status

www.mozilla.org
Product Details
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: sylvestre, Assigned: sylvestre)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

4 years ago
Created attachment 8492236 [details] [diff] [review]
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
Attachment #8492236 - Flags: review?(pascalc)
Attachment #8492236 - Flags: review?(lmandel)
(Assignee)

Updated

4 years ago
Blocks: 1070004
(Assignee)

Comment 1

4 years ago
Created attachment 8492244 [details] [diff] [review]
json-export.diff

Just for information, here is the json file generated with the changes (oh, I <3 the pretty print)
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 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.
Attachment #8492236 - Flags: review?(lmandel) → review+
(Assignee)

Updated

4 years ago
Component: Release Automation → Product Details
Product: Release Engineering → www.mozilla.org
QA Contact: bhearsum
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 :)
Attachment #8492236 - Flags: review?(pascalc) → review+
(Assignee)

Comment 5

4 years ago
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
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
(Assignee)

Updated

4 years ago
Blocks: 1080532
(Assignee)

Updated

3 years ago
Assignee: nobody → sledru
You need to log in before you can comment on or make changes to this bug.