Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2800813003:
Add new method signature to AppIndexingReporter. (Closed)
DescriptionAdd new method signature to AppIndexingReporter.
The old method will be removed in a follow-up cl.
BUG=693633
Review-Url: https://codereview.chromium.org/2800813003
Cr-Commit-Position: refs/heads/master@{#462247}
Committed: https://chromium.googlesource.com/chromium/src/+/574080eabdd7d76bcbffbcae20fbf4bea337a669
Patch Set 1 #
Total comments: 3
Patch Set 2 : only keep new signature #Patch Set 3 : add both signatures #Patch Set 4 : add both signatures #Messages
Total messages: 16 (8 generated)
Description was changed from ========== Add new method signature to AppIndexingReporter. The old method will be removed in a follow-up cl. BUG=693633 ========== to ========== Add new method signature to AppIndexingReporter. The old method will be removed in a follow-up cl. BUG=693633 ==========
dproctor@google.com changed reviewers: + wychen@chromium.org
https://codereview.chromium.org/2800813003/diff/1/chrome/android/java/src/org... File chrome/android/java/src/org/chromium/chrome/browser/historyreport/AppIndexingReporter.java (right): https://codereview.chromium.org/2800813003/diff/1/chrome/android/java/src/org... chrome/android/java/src/org/chromium/chrome/browser/historyreport/AppIndexingReporter.java:29: public void reportWebPage(String url, String json) { The old API should be exactly unchanged. But I see you checked the CQ bit in the downstream CL without using @Override. If that lands first, then it would be like this: - Add the new API downstream without using @Override - Change to the new API upstream - Add @Override downstream
https://codereview.chromium.org/2800813003/diff/1/chrome/android/java/src/org... File chrome/android/java/src/org/chromium/chrome/browser/historyreport/AppIndexingReporter.java (right): https://codereview.chromium.org/2800813003/diff/1/chrome/android/java/src/org... chrome/android/java/src/org/chromium/chrome/browser/historyreport/AppIndexingReporter.java:29: public void reportWebPage(String url, String json) { On 2017/04/05 18:42:22, wychen wrote: > The old API should be exactly unchanged. > > But I see you checked the CQ bit in the downstream CL without using @Override. > If that lands first, then it would be like this: > - Add the new API downstream without using @Override > - Change to the new API upstream > - Add @Override downstream Yeah, sorry, the downstream cq is in progress, and that's the plan.
https://codereview.chromium.org/2800813003/diff/1/chrome/android/java/src/org... File chrome/android/java/src/org/chromium/chrome/browser/historyreport/AppIndexingReporter.java (right): https://codereview.chromium.org/2800813003/diff/1/chrome/android/java/src/org... chrome/android/java/src/org/chromium/chrome/browser/historyreport/AppIndexingReporter.java:29: public void reportWebPage(String url, String json) { On 2017/04/05 19:44:01, dproctor wrote: > On 2017/04/05 18:42:22, wychen wrote: > > The old API should be exactly unchanged. > > > > But I see you checked the CQ bit in the downstream CL without using @Override. > > If that lands first, then it would be like this: > > - Add the new API downstream without using @Override > > - Change to the new API upstream > > - Add @Override downstream > > Yeah, sorry, the downstream cq is in progress, and that's the plan. Update: downstream cq was taking forever and timed out, so I'm changing this back to the original plan: 1. Submit upstream with both signatures. 2. Submit downstream with new method. 3. Submit upstream removing old signature.
dproctor@google.com changed reviewers: + mariakhomenko@chromium.org
lgtm
The CQ bit was checked by wychen@chromium.org
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
Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by dproctor@google.com
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": 60001, "attempt_start_ts": 1491430741172910,
"parent_rev": "968dd8975ab73d3abecd20c171bd4e3a6af7bf71", "commit_rev":
"574080eabdd7d76bcbffbcae20fbf4bea337a669"}
Message was sent while issue was closed.
Description was changed from ========== Add new method signature to AppIndexingReporter. The old method will be removed in a follow-up cl. BUG=693633 ========== to ========== Add new method signature to AppIndexingReporter. The old method will be removed in a follow-up cl. BUG=693633 Review-Url: https://codereview.chromium.org/2800813003 Cr-Commit-Position: refs/heads/master@{#462247} Committed: https://chromium.googlesource.com/chromium/src/+/574080eabdd7d76bcbffbcae20fb... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as https://chromium.googlesource.com/chromium/src/+/574080eabdd7d76bcbffbcae20fb... |
