diagnose_bloat.py: Grouped diffs.

diagnose_bloat.py: Grouped diffs.

Adds grouping by section for resource_sizes diffs to make to diff
output easier to read. Removes summary from output diff file.

resource_sizes diff output:

MonochromePublic.apk_Breakdown (+17,441 bytes)
    +2,769 bytes Non-compiled Android resources size
    +8,712 bytes PNG drawables size
    +1,485 bytes Package metadata size
    +1,995 bytes Java code size
    +2,480 bytes Compiled Android resources size
MonochromePublic.apk_Specifics
   +25,038 bytes normalized apk size
       +20 zip entries file count
    +5,580 bytes main dex size

BUG=733307
Review-Url: https://codereview.chromium.org/2954043002
Cr-Commit-Position: refs/heads/master@{#481946}
Committed: https://chromium.googlesource.com/chromium/src/+/d1dfb45a119293692e31ed824938dae44e4090f9

[estevenson, 2017-06-23 02:15:24]

estevenson@chromium.org changed reviewers:
+ agrieve@chromium.org

[estevenson, 2017-06-23 02:15:25]

ptal Andrew

[agrieve, 2017-06-23 02:40:31]

lgtm. Nice!

As an overall comment, more comments would make it a bit easier to follow. At
the same time, I don't think it's all that important that this code be "nice".
Up to you 🤷‍♂️

https://codereview.chromium.org/2954043002/diff/1/tools/binary_size/diagnose_...
File tools/binary_size/diagnose_bloat.py (right):

https://codereview.chromium.org/2954043002/diff/1/tools/binary_size/diagnose_...
tools/binary_size/diagnose_bloat.py:691: logfile.write('\n'.join(s))
nit: might be simpler to do:

if isinstance(s, basestring):
  data = s.format(*args, **kwargs) + '\n'
else:
  data = '\n'.join(s) + '\n'
sys.stdout.write(data)
logfile.write(data)


That way you'll ensure to write the same thing to the log as you do to stdout
(right now logfile.write is missing a trailing \n in the 2nd case I think.

[estevenson, 2017-06-23 17:23:28]

Felt the same way about comments, added a few since you mentioned it too!
Thanks.

https://codereview.chromium.org/2954043002/diff/1/tools/binary_size/diagnose_...
File tools/binary_size/diagnose_bloat.py (right):

https://codereview.chromium.org/2954043002/diff/1/tools/binary_size/diagnose_...
tools/binary_size/diagnose_bloat.py:691: logfile.write('\n'.join(s))
On 2017/06/23 02:40:30, agrieve wrote:
> nit: might be simpler to do:
> 
> if isinstance(s, basestring):
>   data = s.format(*args, **kwargs) + '\n'
> else:
>   data = '\n'.join(s) + '\n'
> sys.stdout.write(data)
> logfile.write(data)
> 
> 
> That way you'll ensure to write the same thing to the log as you do to stdout
> (right now logfile.write is missing a trailing \n in the 2nd case I think.

Done.

[estevenson, 2017-06-23 17:23:37]

The CQ bit was checked by estevenson@chromium.org

[estevenson, 2017-06-23 17:23:37]

The patchset sent to the CQ was uploaded after l-g-t-m from agrieve@chromium.org
Link to the patchset: https://codereview.chromium.org/2954043002/#ps20001
(title: "review comments")

[commit-bot: I haz the power, 2017-06-23 17:23:53]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-23 17:36:47]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1498238617094250,
"parent_rev": "56acd30f49c6817b8f1dfd1f25255bf6f89460e5", "commit_rev":
"d1dfb45a119293692e31ed824938dae44e4090f9"}

[commit-bot: I haz the power, 2017-06-23 17:37:02]

Description was changed from

==========
diagnose_bloat.py: Grouped diffs.

Adds grouping by section for resource_sizes diffs to make to diff
output easier to read. Removes summary from output diff file.

resource_sizes diff output:

MonochromePublic.apk_Breakdown (+17,441 bytes)
    +2,769 bytes Non-compiled Android resources size
    +8,712 bytes PNG drawables size
    +1,485 bytes Package metadata size
    +1,995 bytes Java code size
    +2,480 bytes Compiled Android resources size
MonochromePublic.apk_Specifics
   +25,038 bytes normalized apk size
       +20 zip entries file count
    +5,580 bytes main dex size

BUG=733307
==========

to

==========
diagnose_bloat.py: Grouped diffs.

Adds grouping by section for resource_sizes diffs to make to diff
output easier to read. Removes summary from output diff file.

resource_sizes diff output:

MonochromePublic.apk_Breakdown (+17,441 bytes)
    +2,769 bytes Non-compiled Android resources size
    +8,712 bytes PNG drawables size
    +1,485 bytes Package metadata size
    +1,995 bytes Java code size
    +2,480 bytes Compiled Android resources size
MonochromePublic.apk_Specifics
   +25,038 bytes normalized apk size
       +20 zip entries file count
    +5,580 bytes main dex size

BUG=733307

Review-Url: https://codereview.chromium.org/2954043002
Cr-Commit-Position: refs/heads/master@{#481946}
Committed:
https://chromium.googlesource.com/chromium/src/+/d1dfb45a119293692e31ed824938...
==========

[commit-bot: I haz the power, 2017-06-23 17:37:03]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/d1dfb45a119293692e31ed824938...