|
|
Created:
3 years, 5 months ago by rnephew (Reviews Here) Modified:
3 years, 4 months ago CC:
catapult-reviews_chromium.org, telemetry-reviews_chromium.org Target Ref:
refs/heads/master Project:
catapult Visibility:
Public. |
Description[Telemetry] Assert that android devices are rooted when running telemetry.
Large portions of the telemetry code base assumes android devices will be rooted.
BUG=catapult:#3672
Review-Url: https://codereview.chromium.org/2980863004
Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/28fb1abdfc1e814c0147c5f2412888a595366a26
Patch Set 1 #Patch Set 2 : Rebase #Messages
Total messages: 23 (13 generated)
The CQ bit was checked by rnephew@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...
rnephew@chromium.org changed reviewers: + charliea@chromium.org, nednguyen@google.com, perezju@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm should an announcement be sent before landing? I wouldn't be surprised if some people are running on user devices because if "sort of works".
On 2017/07/14 15:34:23, perezju wrote: > lgtm > > should an announcement be sent before landing? I wouldn't be surprised if some > people are running on user devices because if "sort of works". +1 lgtm as well
On 2017/07/14 15:37:26, nednguyen wrote: > On 2017/07/14 15:34:23, perezju wrote: > > lgtm > > > > should an announcement be sent before landing? I wouldn't be surprised if some > > people are running on user devices because if "sort of works". > > +1 > > lgtm as well Which mailing list would be appropriate for this announcement? telemetry@chromium? chrome-benchmarks@google? Or is there a better one.
On 2017/07/14 15:44:40, rnephew (Reviews Here) wrote: > On 2017/07/14 15:37:26, nednguyen wrote: > > On 2017/07/14 15:34:23, perezju wrote: > > > lgtm > > > > > > should an announcement be sent before landing? I wouldn't be surprised if > some > > > people are running on user devices because if "sort of works". > > > > +1 > > > > lgtm as well > > Which mailing list would be appropriate for this announcement? > telemetry@chromium? chrome-benchmarks@google? Or is there a better one. Announcement of API changes always go to telemetry-announce@chromium.org. Usually we leave 2 weeks for people to adapt the changes before landing CLs that break the old code contract.
On 2017/07/14 15:46:28, nednguyen wrote: > On 2017/07/14 15:44:40, rnephew (Reviews Here) wrote: > > On 2017/07/14 15:37:26, nednguyen wrote: > > > On 2017/07/14 15:34:23, perezju wrote: > > > > lgtm > > > > > > > > should an announcement be sent before landing? I wouldn't be surprised if > > some > > > > people are running on user devices because if "sort of works". > > > > > > +1 > > > > > > lgtm as well > > > > Which mailing list would be appropriate for this announcement? > > telemetry@chromium? chrome-benchmarks@google? Or is there a better one. > > Announcement of API changes always go to mailto:telemetry-announce@chromium.org. > Usually we leave 2 weeks for people to adapt the changes before landing CLs that > break the old code contract. Should this be landed now?
On 2017/08/17 08:12:56, perezju wrote: > On 2017/07/14 15:46:28, nednguyen wrote: > > On 2017/07/14 15:44:40, rnephew (Reviews Here) wrote: > > > On 2017/07/14 15:37:26, nednguyen wrote: > > > > On 2017/07/14 15:34:23, perezju wrote: > > > > > lgtm > > > > > > > > > > should an announcement be sent before landing? I wouldn't be surprised > if > > > some > > > > > people are running on user devices because if "sort of works". > > > > > > > > +1 > > > > > > > > lgtm as well > > > > > > Which mailing list would be appropriate for this announcement? > > > telemetry@chromium? chrome-benchmarks@google? Or is there a better one. > > > > Announcement of API changes always go to > mailto:telemetry-announce@chromium.org. > > Usually we leave 2 weeks for people to adapt the changes before landing CLs > that > > break the old code contract. > > Should this be landed now? Oh. Yeah. I'll land it today. That two week waiting period made me forget all about this. I'll send out an email now for a last chance for people to speak up, and then land it.
The CQ bit was checked by rnephew@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...
On 2017/08/17 15:08:19, rnephew (Reviews Here) wrote: > On 2017/08/17 08:12:56, perezju wrote: > > On 2017/07/14 15:46:28, nednguyen wrote: > > > On 2017/07/14 15:44:40, rnephew (Reviews Here) wrote: > > > > On 2017/07/14 15:37:26, nednguyen wrote: > > > > > On 2017/07/14 15:34:23, perezju wrote: > > > > > > lgtm > > > > > > > > > > > > should an announcement be sent before landing? I wouldn't be surprised > > if > > > > some > > > > > > people are running on user devices because if "sort of works". > > > > > > > > > > +1 > > > > > > > > > > lgtm as well > > > > > > > > Which mailing list would be appropriate for this announcement? > > > > telemetry@chromium? chrome-benchmarks@google? Or is there a better one. > > > > > > Announcement of API changes always go to > > mailto:telemetry-announce@chromium.org. > > > Usually we leave 2 weeks for people to adapt the changes before landing CLs > > that > > > break the old code contract. > > > > Should this be landed now? > > Oh. Yeah. I'll land it today. That two week waiting period made me forget all > about this. I'll send out an email now for a last chance for people to speak up, > and then land it. So 2 weeks turned into a month, but I'm going to land this today. I sent out a reply on the email saying I was going to do this telling everyone its happening today. I just started a dry run on this. If it passes, I'll land after lunch.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by rnephew@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from nednguyen@google.com, perezju@chromium.org Link to the patchset: https://codereview.chromium.org/2980863004/#ps20001 (title: "Rebase")
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": 20001, "attempt_start_ts": 1503336629437190, "parent_rev": "68a3cd4a072c5d4cbae1c59833dea32c669d2bc4", "commit_rev": "28fb1abdfc1e814c0147c5f2412888a595366a26"}
Message was sent while issue was closed.
Description was changed from ========== [Telemetry] Assert that android devices are rooted when running telemetry. Large portions of the telemetry code base assumes android devices will be rooted. BUG=catapult:#3672 ========== to ========== [Telemetry] Assert that android devices are rooted when running telemetry. Large portions of the telemetry code base assumes android devices will be rooted. BUG=catapult:#3672 Review-Url: https://codereview.chromium.org/2980863004 Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapu... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/external/github.com/catapult-project/catapu... |