Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2202333002:
Remove page_cycler.top_10_mobile. (Closed)
DescriptionRemove page_cycler.top_10_mobile. Already ported to page_cycler_v2.
R=nednguyen@google.com
BUG=633222
Committed: https://crrev.com/7604a139438e4170a2247043679c0d044232b507
Cr-Commit-Position: refs/heads/master@{#409294}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Completely remove benchmark. #Messages
Total messages: 20 (9 generated)
Description was changed from ========== Disable page_cycler.top_10_mobile on android. R=nednyugen@google.com BUG=633222 ========== to ========== Disable page_cycler.top_10_mobile on android. R=nednyugen@google.com BUG=633222 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:android_s5_perf_cq;master.tryserver.chromium.perf:linux_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq;master.tryserver.chromium.perf:winx64_10_perf_cq ==========
Description was changed from ========== Disable page_cycler.top_10_mobile on android. R=nednyugen@google.com BUG=633222 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:android_s5_perf_cq;master.tryserver.chromium.perf:linux_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq;master.tryserver.chromium.perf:winx64_10_perf_cq ========== to ========== Disable page_cycler.top_10_mobile on android. R=nednguyen@google.com BUG=633222 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:android_s5_perf_cq;master.tryserver.chromium.perf:linux_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq;master.tryserver.chromium.perf:winx64_10_perf_cq ==========
simonhatch@chromium.org changed reviewers: + nednguyen@google.com - nednyugen@google.com
https://codereview.chromium.org/2202333002/diff/1/tools/perf/benchmarks/page_... File tools/perf/benchmarks/page_cycler.py (right): https://codereview.chromium.org/2202333002/diff/1/tools/perf/benchmarks/page_... tools/perf/benchmarks/page_cycler.py:120: @benchmark.Disabled('all') # crbug.com/633222 Is this right? This is kinda weird, since enabled just enables for the specified targets right?
https://codereview.chromium.org/2202333002/diff/1/tools/perf/benchmarks/page_... File tools/perf/benchmarks/page_cycler.py (right): https://codereview.chromium.org/2202333002/diff/1/tools/perf/benchmarks/page_... tools/perf/benchmarks/page_cycler.py:120: @benchmark.Disabled('all') # crbug.com/633222 On 2016/08/02 16:02:14, shatch wrote: > Is this right? This is kinda weird, since enabled just enables for the specified > targets right? Yeah, Enabled is white-listing, whereas Disabled is black-listing.
lgtm
nednguyen@google.com changed reviewers: + kouhei@chromium.org
Actually, the right action here is to delete this benchmark entirely.
On 2016/08/02 16:55:49, nednguyen wrote: > Actually, the right action here is to delete this benchmark entirely. Ah, saw your comment in the bug that this was already ported to page_cycler_v2, removed it. Will wait before submitting.
On 2016/08/02 17:22:38, shatch wrote: > On 2016/08/02 16:55:49, nednguyen wrote: > > Actually, the right action here is to delete this benchmark entirely. > > Ah, saw your comment in the bug that this was already ported to page_cycler_v2, > removed it. Will wait before submitting. Update the title? :-) lgtm
Description was changed from ========== Disable page_cycler.top_10_mobile on android. R=nednguyen@google.com BUG=633222 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:android_s5_perf_cq;master.tryserver.chromium.perf:linux_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq;master.tryserver.chromium.perf:winx64_10_perf_cq ========== to ========== Remove page_cycler.top_10_mobile. Already ported to page_cycler_v2. R=nednguyen@google.com BUG=633222 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:android_s5_perf_cq;master.tryserver.chromium.perf:linux_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq;master.tryserver.chromium.perf:winx64_10_perf_cq ==========
On 2016/08/02 17:43:14, nednguyen wrote: > On 2016/08/02 17:22:38, shatch wrote: > > On 2016/08/02 16:55:49, nednguyen wrote: > > > Actually, the right action here is to delete this benchmark entirely. > > > > Ah, saw your comment in the bug that this was already ported to > page_cycler_v2, > > removed it. Will wait before submitting. > > Update the title? :-) > > > lgtm Oops yes!
Description was changed from ========== Remove page_cycler.top_10_mobile. Already ported to page_cycler_v2. R=nednguyen@google.com BUG=633222 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:android_s5_perf_cq;master.tryserver.chromium.perf:linux_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq;master.tryserver.chromium.perf:winx64_10_perf_cq ========== to ========== Remove page_cycler.top_10_mobile. Already ported to page_cycler_v2. R=nednguyen@google.com BUG=633222 ==========
The CQ bit was checked by simonhatch@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Remove page_cycler.top_10_mobile. Already ported to page_cycler_v2. R=nednguyen@google.com BUG=633222 ========== to ========== Remove page_cycler.top_10_mobile. Already ported to page_cycler_v2. R=nednguyen@google.com BUG=633222 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Remove page_cycler.top_10_mobile. Already ported to page_cycler_v2. R=nednguyen@google.com BUG=633222 ========== to ========== Remove page_cycler.top_10_mobile. Already ported to page_cycler_v2. R=nednguyen@google.com BUG=633222 Committed: https://crrev.com/7604a139438e4170a2247043679c0d044232b507 Cr-Commit-Position: refs/heads/master@{#409294} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/7604a139438e4170a2247043679c0d044232b507 Cr-Commit-Position: refs/heads/master@{#409294}
Message was sent while issue was closed.
lgtm |
