1068049 - Pretty print the json exported file by the product details

Status: RESOLVED FIXED

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

Description

[Sylvestre Ledru [:Sylvestre]]

Attachment: pretty-print-json-export.diff

Currently, the json files are stored on a single line.
This has two side effects:
* hard/impossible to see the changes in a single diff
* big diff for each svn commit

The attached patch proposed to use the pretty print option of the PHP json exporter.

Comment 1

[Sylvestre Ledru [:Sylvestre]]

Attachment: json-export.diff

Example of the regenerated json files

Comment 2

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

An advantage of the current structure is less white space and therefore smaller file size. I take it this file is used at runtime where these things matter but am happy to be corrected if that's not the case.

Comment 3

[Sylvestre Ledru [:Sylvestre]]

I am not sure how whitespaces impact the overall performance but I hope it is negligible.

With the current data, 1084k without spaces, 1268k with spaces.

n-i our www friends :)

Comment 4

[Paul [:pmac] McLanahan]

I guess it doesn't matter... though I'm not sure the point of this. Why do you need to see the JSON diffs? The canonical data and changes are still in the PHP right? This is just a side effect that I'd hope can be assumed to be correct.

However, at least the way we typically use the JSON, it shouldn't matter. We load it on the server side into memory and use the data, so we never send this JSON to the client. I suspect anyone who wanted to use this client side could at least have it served gzipped, and that would make the extra space moot.

Comment 5

[Sylvestre Ledru [:Sylvestre]]

I am doing changes on product details. Having a pretty print version of the json helps to keep track of potential issues. The data/order remain the same.

Comment 6

[Josh Mize [:jgmize]]

Agreed with :pmac, the performance impact is negligible, so if it helps troubleshooting then it sounds like a good change to me.

Comment 7

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

Comment on attachment 8490099 [details] [diff] [review]
pretty-print-json-export.diff

r+ given that this won't impact runtime perf.

Comment 8

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8490099 [details] [diff] [review]
pretty-print-json-export.diff

Committed here:
http://viewvc.svn.mozilla.org/vc/libs/product-details/export_json.php?r1=130443&r2=131941
Regenerated all json files here:
http://viewvc.svn.mozilla.org/vc?view=revision&revision=131942