|
|
Chromium Code Reviews
DescriptionPhoto Picker dialog: Add UMA for file enumeration.
BUG=656015
Review-Url: https://codereview.chromium.org/2933173003
Cr-Commit-Position: refs/heads/master@{#479162}
Committed: https://chromium.googlesource.com/chromium/src/+/9fc9d951c4b4757038d8ae9dbeafd43bea88aab3
Patch Set 1 #
Total comments: 3
Messages
Total messages: 26 (18 generated)
Description was changed from ========== Photo Picker dialog: Add UMA for file enumeration. BUG=656015 ========== to ========== Photo Picker dialog: Add UMA for file enumeration. BUG=656015 ==========
finnur@chromium.org changed reviewers: + twellington@chromium.org
finnur@chromium.org changed reviewers: + isherman@chromium.org
PTAL
lgtm https://codereview.chromium.org/2933173003/diff/1/chrome/android/java/src/org... File chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java (right): https://codereview.chromium.org/2933173003/diff/1/chrome/android/java/src/org... chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java:224: int rate = (int) (100 * files.size() / elapsedTimeMs); nit: Add a comment here that enumerated rate is per 10th of a second so that the its more obvious why we're multiplying by 100
https://codereview.chromium.org/2933173003/diff/1/chrome/android/java/src/org... File chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java (right): https://codereview.chromium.org/2933173003/diff/1/chrome/android/java/src/org... chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java:224: int rate = (int) (100 * files.size() / elapsedTimeMs); Should I move the comment on line 222 down one line or reword it to make it clearer?
The CQ bit was checked by finnur@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2933173003/diff/1/chrome/android/java/src/org... File chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java (right): https://codereview.chromium.org/2933173003/diff/1/chrome/android/java/src/org... chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java:224: int rate = (int) (100 * files.size() / elapsedTimeMs); On 2017/06/13 15:53:04, Finnur wrote: > Should I move the comment on line 222 down one line or reword it to make it > clearer? Oh.. No. The comment you have is fine. Sorry.
No problem. Thanks for the review! Ilya, if this is worthy of the green light, then feel free to tick the CQ box.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by finnur@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
The CQ bit was checked by finnur@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Metrics LGTM, thanks.
The CQ bit was checked by isherman@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 1, "attempt_start_ts": 1497389539523160, "parent_rev":
"eaa976b4923b9c257b9d79912a852403cffaf3f0", "commit_rev":
"9fc9d951c4b4757038d8ae9dbeafd43bea88aab3"}
Message was sent while issue was closed.
Description was changed from ========== Photo Picker dialog: Add UMA for file enumeration. BUG=656015 ========== to ========== Photo Picker dialog: Add UMA for file enumeration. BUG=656015 Review-Url: https://codereview.chromium.org/2933173003 Cr-Commit-Position: refs/heads/master@{#479162} Committed: https://chromium.googlesource.com/chromium/src/+/9fc9d951c4b4757038d8ae9dbeaf... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/9fc9d951c4b4757038d8ae9dbeaf... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
