Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(261)

Issue 2847243005: diagnose_apk_bloat.py: fix error messages and simplify rev order. (Closed)

Created:
3 years, 7 months ago by estevenson
Modified:
3 years, 7 months ago
Reviewers:
agrieve
CC:
chromium-reviews, wnwen+watch_chromium.org, estevenson+watch_chromium.org, agrieve+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

diagnose_apk_bloat.py: fix error messages and simplify rev order. * Improve error messages related to supplying invalid rev or --reference-rev args * Change diff classes to use "before" and "after" instead of a list of dirs to make the code a little easier to follow * Fix Linux issues and make --cloud work for Linux BUG=695182 Review-Url: https://codereview.chromium.org/2847243005 Cr-Commit-Position: refs/heads/master@{#468690} Committed: https://chromium.googlesource.com/chromium/src/+/c0fd70a81b94dcd6c976b51fab798c0178a33613

Patch Set 1 #

Patch Set 2 : fix comment #

Patch Set 3 : + linux updates #

Patch Set 4 : + README #

Total comments: 2

Patch Set 5 : Linux comment #

Patch Set 6 : diagnose_apk_bloat.py: fix error messages and simplify rev order. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+118 lines, -100 lines) Patch
M tools/binary_size/README.md View 1 2 3 4 5 2 chunks +3 lines, -2 lines 0 comments Download
M tools/binary_size/diagnose_apk_bloat.py View 1 2 3 4 14 chunks +115 lines, -98 lines 0 comments Download

Messages

Total messages: 12 (7 generated)
estevenson
3 years, 7 months ago (2017-05-01 23:02:41 UTC) #4
agrieve
Nice cleanup. just one nit. lgtm https://codereview.chromium.org/2847243005/diff/60001/tools/binary_size/diagnose_apk_bloat.py File tools/binary_size/diagnose_apk_bloat.py (right): https://codereview.chromium.org/2847243005/diff/60001/tools/binary_size/diagnose_apk_bloat.py#newcode739 tools/binary_size/diagnose_apk_bloat.py:739: _VerifyUserAccepts('Linux diffs are ...
3 years, 7 months ago (2017-05-02 00:16:52 UTC) #5
estevenson
https://codereview.chromium.org/2847243005/diff/60001/tools/binary_size/diagnose_apk_bloat.py File tools/binary_size/diagnose_apk_bloat.py (right): https://codereview.chromium.org/2847243005/diff/60001/tools/binary_size/diagnose_apk_bloat.py#newcode739 tools/binary_size/diagnose_apk_bloat.py:739: _VerifyUserAccepts('Linux diffs are less useful than Android diffs, ' ...
3 years, 7 months ago (2017-05-02 16:00:10 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2847243005/100001
3 years, 7 months ago (2017-05-02 17:01:14 UTC) #9
commit-bot: I haz the power
3 years, 7 months ago (2017-05-02 17:11:43 UTC) #12
Message was sent while issue was closed.
Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/c0fd70a81b94dcd6c976b51fab79...

Powered by Google App Engine
This is Rietveld 408576698