New issue
Advanced search Search tips

Issue 914654 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 9
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

android-binary-size UX tweaks

Project Member Reported by agrieve@chromium.org, Dec 13

Issue description

1. Show all diffs under the same step (so they aren't so spread out)
2. Surface method count diff without having to click into details

 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 21

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/59ec80f93985216091996e8b39f45ba4da8ea4f4

commit 59ec80f93985216091996e8b39f45ba4da8ea4f4
Author: Andrew Grieve <agrieve@chromium.org>
Date: Fri Dec 21 18:51:10 2018

trybot_commit_size_checker.py: Communicate more things via JSON

This change maintains compatibility with existing recipe, but adds extra
JSON entries to be used by the next recipe version.

Additions:
* A "summary" blurb to render directly below step name.
* List of files to archive
* Inline diff results (rather than temporary files).
* List of links to show (for each diff result).

Bug:  914654 
Change-Id: I1b44a52cc6266ae6f1172b2a0852794ed80e0e23
Reviewed-on: https://chromium-review.googlesource.com/c/1388292
Reviewed-by: Eric Stevenson <estevenson@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#618562}
[modify] https://crrev.com/59ec80f93985216091996e8b39f45ba4da8ea4f4/tools/binary_size/trybot_commit_size_checker.py

Related: What do you think about running the trybot even when there's a "Binary-Size:" footer? Might still be useful to be able to check added methods/size, especially when there's a downstream alert?
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 3

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/a6b42c570e7e118fa69b17916b69c6b453a9c59d

commit a6b42c570e7e118fa69b17916b69c6b453a9c59d
Author: Andrew Grieve <agrieve@chromium.org>
Date: Thu Jan 03 14:50:10 2019

binary_size_trybot.py: Update results.json format

New format moves more of the logic to trybot_commit_size_checker.py.
The result.json from this src-side script now contains:
* Step summary text (shown as step_text)
* List of files to archive
* List of result links

Hoping this will be a bit easier to grok as all interesting links are
new grouped under the same step.

Bug:  914654 
Change-Id: Id7267f1ef5864f9af1334932afd0503f44b0e04f
Reviewed-on: https://chromium-review.googlesource.com/c/1387912
Commit-Queue: agrieve <agrieve@chromium.org>
Reviewed-by: John Budorick <jbudorick@chromium.org>

[modify] https://crrev.com/a6b42c570e7e118fa69b17916b69c6b453a9c59d/scripts/slave/README.recipes.md
[modify] https://crrev.com/a6b42c570e7e118fa69b17916b69c6b453a9c59d/scripts/slave/recipes/binary_size_trybot.expected/unexpected_increase.json
[modify] https://crrev.com/a6b42c570e7e118fa69b17916b69c6b453a9c59d/scripts/slave/recipes/binary_size_trybot.expected/normal_build.json
[delete] https://crrev.com/f23137dbac11445e394376ea21362e867dd33f2b/scripts/slave/recipes/binary_size_trybot.expected/malformed_results_json.json
[modify] https://crrev.com/a6b42c570e7e118fa69b17916b69c6b453a9c59d/scripts/slave/recipes/binary_size_trybot.py

Status: Fixed (was: Started)
re #2 - I think it's probably a good idea at this point to just always run it. I've generally found that for reviews that disabled it, it was no disabled in past patchsets, but it would be nice if the size diff was available for the final patchset.

Marking this as fixed. since the UI is now updated.
Project Member

Comment 5 by bugdroid1@chromium.org, Jan 11

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/980dc087e522f1cf1a653a0769802be35b269ba6

commit 980dc087e522f1cf1a653a0769802be35b269ba6
Author: Andrew Grieve <agrieve@chromium.org>
Date: Fri Jan 11 21:30:06 2019

trybot_commit_size_checker.py: Show resource_sizes.py diff inline

* Removes obsolete command-line flags.
* Moves resource_sizes.py diff as well as failing size checks inline

Bug:  914654 
Change-Id: I444da444a2bbed4a154ce1d4a64bb99549e81e86
Reviewed-on: https://chromium-review.googlesource.com/c/1407375
Reviewed-by: Eric Stevenson <estevenson@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622156}
[modify] https://crrev.com/980dc087e522f1cf1a653a0769802be35b269ba6/tools/binary_size/diagnose_bloat.py
[modify] https://crrev.com/980dc087e522f1cf1a653a0769802be35b269ba6/tools/binary_size/trybot_commit_size_checker.py

Sign in to add a comment