Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2790313002:
Add chooser watchlist and add ortuno to it (Closed)
DescriptionAdd chooser watchlist and add ortuno to it
Review-Url: https://codereview.chromium.org/2790313002
Cr-Commit-Position: refs/heads/master@{#461920}
Committed: https://chromium.googlesource.com/chromium/src/+/e624054d06931cb30017455519b924027d97320c
Patch Set 1 #
Total comments: 4
Patch Set 2 : git upstream-diff #Patch Set 3 : Fix #Messages
Total messages: 15 (7 generated)
ortuno@chromium.org changed reviewers: + juncai@chromium.org
juncai: PTAL
https://codereview.chromium.org/2790313002/diff/1/WATCHLISTS File WATCHLISTS (right): https://codereview.chromium.org/2790313002/diff/1/WATCHLISTS#newcode40 WATCHLISTS:40: 'android_chooser_dialogs': { 'android_chooser_dialogs' seems too general, what about "android_device_chooser_dialogs"? We use 'device_chooser' for desktops: https://cs.chromium.org/chromium/src/chrome/browser/ui/views/device_chooser_c...
https://codereview.chromium.org/2790313002/diff/1/WATCHLISTS File WATCHLISTS (right): https://codereview.chromium.org/2790313002/diff/1/WATCHLISTS#newcode40 WATCHLISTS:40: 'android_chooser_dialogs': { On 2017/04/04 at 00:26:46, juncai wrote: > 'android_chooser_dialogs' seems too general, what about "android_device_chooser_dialogs"? We use 'device_chooser' for desktops: > https://cs.chromium.org/chromium/src/chrome/browser/ui/views/device_chooser_c... android_device_chooser_dialogs would limit us to UsbChooserDialog and BluetoothChooserDialog but I actually want to watch the more general ItemChooserDialog.
https://codereview.chromium.org/2790313002/diff/1/WATCHLISTS File WATCHLISTS (right): https://codereview.chromium.org/2790313002/diff/1/WATCHLISTS#newcode40 WATCHLISTS:40: 'android_chooser_dialogs': { On 2017/04/04 00:35:37, ortuno wrote: > On 2017/04/04 at 00:26:46, juncai wrote: > > 'android_chooser_dialogs' seems too general, what about > "android_device_chooser_dialogs"? We use 'device_chooser' for desktops: > > > https://cs.chromium.org/chromium/src/chrome/browser/ui/views/device_chooser_c... > > android_device_chooser_dialogs would limit us to UsbChooserDialog and > BluetoothChooserDialog but I actually want to watch the more general > ItemChooserDialog. hmm... still feel that the 'android_chooser_dialogs' is too general since there are other choosers on Android: https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chr... https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chr... What about 'android_item_chooser_dialogs'? Or other name that can make it more specific.
https://codereview.chromium.org/2790313002/diff/1/WATCHLISTS File WATCHLISTS (right): https://codereview.chromium.org/2790313002/diff/1/WATCHLISTS#newcode40 WATCHLISTS:40: 'android_chooser_dialogs': { On 2017/04/04 at 00:46:16, juncai wrote: > On 2017/04/04 00:35:37, ortuno wrote: > > On 2017/04/04 at 00:26:46, juncai wrote: > > > 'android_chooser_dialogs' seems too general, what about > > "android_device_chooser_dialogs"? We use 'device_chooser' for desktops: > > > > > https://cs.chromium.org/chromium/src/chrome/browser/ui/views/device_chooser_c... > > > > android_device_chooser_dialogs would limit us to UsbChooserDialog and > > BluetoothChooserDialog but I actually want to watch the more general > > ItemChooserDialog. > > hmm... still feel that the 'android_chooser_dialogs' is too general since there are other choosers on Android: > https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chr... > https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chr... > What about 'android_item_chooser_dialogs'? Or other name that can make it more specific. I see. Done.
LGTM.
The CQ bit was checked by ortuno@chromium.org
The CQ bit was unchecked by ortuno@chromium.org
The CQ bit was checked by ortuno@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from juncai@chromium.org Link to the patchset: https://codereview.chromium.org/2790313002/#ps40001 (title: "Fix")
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": 40001, "attempt_start_ts": 1491351681554510,
"parent_rev": "e3f640d2cbfc90e0c0c5eb7ae5662cdb05aff9b4", "commit_rev":
"e624054d06931cb30017455519b924027d97320c"}
Message was sent while issue was closed.
Description was changed from ========== Add chooser watchlist and add ortuno to it ========== to ========== Add chooser watchlist and add ortuno to it Review-Url: https://codereview.chromium.org/2790313002 Cr-Commit-Position: refs/heads/master@{#461920} Committed: https://chromium.googlesource.com/chromium/src/+/e624054d06931cb30017455519b9... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/e624054d06931cb30017455519b9... |
