Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
14021003:
Rework the deps so that we pull all of Blink into third_party/WebKit (Closed)
|
Created:
7 years, 8 months ago by Dirk Pranke Modified:
7 years, 8 months ago CC:
chromium-reviews, kerz_chromium, laforge, karen Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
DescriptionRework the deps so that we pull all of Blink into third_party/WebKit
R=abarth@chromium.org, iannucci@chromium.org, cmp@chromium.org
BUG=229611
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=195325
Patch Set 1 #
Total comments: 8
Patch Set 2 : fix typo, add TODO about renaming webkit_* vars #Patch Set 3 : update, rebase #Messages
Total messages: 22 (0 generated)
https://codereview.chromium.org/14021003/diff/1/DEPS File DEPS (right): https://codereview.chromium.org/14021003/diff/1/DEPS#newcode9 DEPS:9: "webkit_trunk": "http://src.chromium.org/blink/trunk", Is this still the version we should be using for webkit_trunk? Should I add a TODO to rename this to "blink_trunk", and is it still needed?
I'm very happy we're making this change. I have no idea what consequences it has. :) https://codereview.chromium.org/14021003/diff/1/DEPS File DEPS (right): https://codereview.chromium.org/14021003/diff/1/DEPS#newcode11 DEPS:11: "webkit_revision": "148098", We should also rename this to blink_revision at some point. https://codereview.chromium.org/14021003/diff/1/DEPS#newcode184 DEPS:184: "rc/content/test/data/layout_tests/LayoutTests/fast/js/resources": Looks like you've got a stray edit here.
https://codereview.chromium.org/14021003/diff/1/DEPS File DEPS (left): https://codereview.chromium.org/14021003/diff/1/DEPS#oldcode52 DEPS:52: "ios_webkit_trunk": "http://svn.webkit.org/repository/webkit/trunk", Should this point to blink as well?
https://codereview.chromium.org/14021003/diff/1/DEPS File DEPS (left): https://codereview.chromium.org/14021003/diff/1/DEPS#oldcode52 DEPS:52: "ios_webkit_trunk": "http://svn.webkit.org/repository/webkit/trunk", On 2013/04/11 04:05:27, abarth wrote: > Should this point to blink as well? No, this should be left as-is for now. https://codereview.chromium.org/14021003/diff/1/DEPS File DEPS (right): https://codereview.chromium.org/14021003/diff/1/DEPS#newcode9 DEPS:9: "webkit_trunk": "http://src.chromium.org/blink/trunk", On 2013/04/11 03:41:57, Dirk Pranke wrote: > Is this still the version we should be using for webkit_trunk? Should I add a > TODO to rename this to "blink_trunk", and is it still needed? TODO is fine, but please don't change the var name at this time. https://codereview.chromium.org/14021003/diff/1/DEPS#newcode11 DEPS:11: "webkit_revision": "148098", On 2013/04/11 04:03:16, abarth wrote: > We should also rename this to blink_revision at some point. I recommend not changing this at this time. Other scripts rewrite these variables and override these entries. They'll all need to be updated, too, in conjunction.
https://codereview.chromium.org/14021003/diff/1/DEPS File DEPS (right): https://codereview.chromium.org/14021003/diff/1/DEPS#newcode11 DEPS:11: "webkit_revision": "148098", On 2013/04/11 04:16:45, cmp wrote: > On 2013/04/11 04:03:16, abarth wrote: > > We should also rename this to blink_revision at some point. > > I recommend not changing this at this time. Other scripts rewrite these > variables and override these entries. They'll all need to be updated, too, in > conjunction. Make sense.
On 2013/04/11 04:22:45, abarth wrote: > https://codereview.chromium.org/14021003/diff/1/DEPS > File DEPS (right): > > https://codereview.chromium.org/14021003/diff/1/DEPS#newcode11 > DEPS:11: "webkit_revision": "148098", > On 2013/04/11 04:16:45, cmp wrote: > > On 2013/04/11 04:03:16, abarth wrote: > > > We should also rename this to blink_revision at some point. > > > > I recommend not changing this at this time. Other scripts rewrite these > > variables and override these entries. They'll all need to be updated, too, in > > conjunction. > > Make sense. Makes sense :)
Please take another look? I'd like to land this fairly early in the afternoon if possible so we can hammer the server before it gets too late in the day.
On 2013/04/11 19:12:25, Dirk Pranke wrote: > Please take another look? I'd like to land this fairly early in the afternoon if > possible so we can hammer the server before it gets too late in the day. Note that this patch should not land until we get a green light from the infrastructure team ...
This change LGTM in that I would like the DEPS file to have the post-patch contents, but I don't have any appreciation for what will explode when we land this CL.
On 2013/04/12 02:43:27, abarth wrote: > This change LGTM in that I would like the DEPS file to have the post-patch > contents, but I don't have any appreciation for what will explode when we land > this CL. The explosion would mostly be due to a fleet-wide clobber of the blink checkout. We're working on bringing up some fancy new hardware to serve as the golo svn mirror, but we hit a snag along the way. As soon as the new golo mirror is up and running, it should be safe to land this patch :).
On 2013/04/12 02:58:01, iannucci wrote: > On 2013/04/12 02:43:27, abarth wrote: > > This change LGTM in that I would like the DEPS file to have the post-patch > > contents, but I don't have any appreciation for what will explode when we land > > this CL. > > The explosion would mostly be due to a fleet-wide clobber of the blink checkout. > We're working on bringing up some fancy new hardware to serve as the golo svn > mirror, but we hit a snag along the way. As soon as the new golo mirror is up > and running, it should be safe to land this patch :). Any chance we can roll this out Sunday night or Monday? I want to make sure we don't have to do anything for Canaries if this busts things over the weekend.
On 2013/04/12 22:11:36, kerz_google wrote: > On 2013/04/12 02:58:01, iannucci wrote: > > On 2013/04/12 02:43:27, abarth wrote: > > > This change LGTM in that I would like the DEPS file to have the post-patch > > > contents, but I don't have any appreciation for what will explode when we > land > > > this CL. > > > > The explosion would mostly be due to a fleet-wide clobber of the blink > checkout. > > We're working on bringing up some fancy new hardware to serve as the golo svn > > mirror, but we hit a snag along the way. As soon as the new golo mirror is up > > and running, it should be safe to land this patch :). > > Any chance we can roll this out Sunday night or Monday? I want to make sure we > don't have to do anything for Canaries if this busts things over the weekend. At this point I'm not expecting to land this before Monday at the earliest.
I'm thinking we won't roll this out until at least monday... I'm assuming later is better? On Fri, Apr 12, 2013 at 3:11 PM, <kerz@google.com> wrote: > On 2013/04/12 02:58:01, iannucci wrote: > >> On 2013/04/12 02:43:27, abarth wrote: >> > This change LGTM in that I would like the DEPS file to have the >> post-patch >> > contents, but I don't have any appreciation for what will explode when >> we >> > land > >> > this CL. >> > > The explosion would mostly be due to a fleet-wide clobber of the blink >> > checkout. > >> We're working on bringing up some fancy new hardware to serve as the golo >> svn >> mirror, but we hit a snag along the way. As soon as the new golo mirror >> is up >> and running, it should be safe to land this patch :). >> > > Any chance we can roll this out Sunday night or Monday? I want to make > sure we > don't have to do anything for Canaries if this busts things over the > weekend. > > https://codereview.chromium.**org/14021003/<https://codereview.chromium.org/1... >
lgtm Let's plan to roll this out either Wed Apr 17 or Thu Apr 18, landing sometime around 6PM.
On 2013/04/17 02:24:18, cmp_google wrote: > lgtm > > Let's plan to roll this out either Wed Apr 17 or Thu Apr 18, landing sometime > around 6PM. Okay, looks like the try jobs are more-or-less happy and the subsequent try jobs are reverting more-or-less correctly, e.g.: http://build.chromium.org/p/tryserver.chromium/buildslaves/slave322-c4 http://build.chromium.org/p/tryserver.chromium/builders/linux_clang/builds/10... http://build.chromium.org/p/tryserver.chromium/builders/linux_clang/builds/10... Currently plan is to land this around 4pm or 4:30pm today (US/Pacific time)
On 2013/04/19 20:45:53, Dirk Pranke wrote: > On 2013/04/17 02:24:18, cmp_google wrote: > > lgtm > > > > Let's plan to roll this out either Wed Apr 17 or Thu Apr 18, landing sometime > > around 6PM. > > Okay, looks like the try jobs are more-or-less happy and the subsequent try jobs > are reverting more-or-less correctly, e.g.: > > http://build.chromium.org/p/tryserver.chromium/buildslaves/slave322-c4 > > http://build.chromium.org/p/tryserver.chromium/builders/linux_clang/builds/10... > http://build.chromium.org/p/tryserver.chromium/builders/linux_clang/builds/10... > > Currently plan is to land this around 4pm or 4:30pm today (US/Pacific time) My same point from last week stands, can we hold off on doing this in case we have an issue with Canaries until Monday when there are people around to triage? This weekend is especially bad as three of the four TPMs will be offline.
On 2013/04/19 20:51:11, kerz_google wrote: > On 2013/04/19 20:45:53, Dirk Pranke wrote: > > On 2013/04/17 02:24:18, cmp_google wrote: > > > lgtm > > > > > > Let's plan to roll this out either Wed Apr 17 or Thu Apr 18, landing > sometime > > > around 6PM. > > > > Okay, looks like the try jobs are more-or-less happy and the subsequent try > jobs > > are reverting more-or-less correctly, e.g.: > > > > http://build.chromium.org/p/tryserver.chromium/buildslaves/slave322-c4 > > > > > http://build.chromium.org/p/tryserver.chromium/builders/linux_clang/builds/10... > > > http://build.chromium.org/p/tryserver.chromium/builders/linux_clang/builds/10... > > > > Currently plan is to land this around 4pm or 4:30pm today (US/Pacific time) > > My same point from last week stands, can we hold off on doing this in case we > have an issue with Canaries until Monday when there are people around to triage? > This weekend is especially bad as three of the four TPMs will be offline. At this point, I would prefer not to hold off any longer on this change. There are a set of fixes we need to get in on the infrastructure to try and stabilize things for Blink dev and the try bots, and I was hoping to get some time this weekend to spend working on them. Is there something we can do (or help with) to alleviate concerns about the canaries?
Karen is the point person for this weekend, as she'll be running it I believe, so I leave it up to her. On Fri, Apr 19, 2013 at 1:55 PM, <dpranke@chromium.org> wrote: > On 2013/04/19 20:51:11, kerz_google wrote: > >> On 2013/04/19 20:45:53, Dirk Pranke wrote: >> > On 2013/04/17 02:24:18, cmp_google wrote: >> > > lgtm >> > > >> > > Let's plan to roll this out either Wed Apr 17 or Thu Apr 18, landing >> sometime >> > > around 6PM. >> > >> > Okay, looks like the try jobs are more-or-less happy and the subsequent >> try >> jobs >> > are reverting more-or-less correctly, e.g.: >> > >> > http://build.chromium.org/p/**tryserver.chromium/** >> buildslaves/slave322-c4<http://build.chromium.org/p/tryserver.chromium/buildslaves/slave322-c4> >> > >> > >> > > http://build.chromium.org/p/**tryserver.chromium/builders/** > linux_clang/builds/100220<http://build.chromium.org/p/tryserver.chromium/builders/linux_clang/builds/100220> > >> > >> > > http://build.chromium.org/p/**tryserver.chromium/builders/** > linux_clang/builds/100229<http://build.chromium.org/p/tryserver.chromium/builders/linux_clang/builds/100229> > >> > >> > Currently plan is to land this around 4pm or 4:30pm today (US/Pacific >> time) >> > > My same point from last week stands, can we hold off on doing this in >> case we >> have an issue with Canaries until Monday when there are people around to >> > triage? > >> This weekend is especially bad as three of the four TPMs will be >> offline. >> > > At this point, I would prefer not to hold off any longer on this change. > There > are a set of fixes we need to get in on the infrastructure to try and > stabilize > things for Blink dev and the try bots, and I was hoping to get some time > this > weekend to spend working on them. > > Is there something we can do (or help with) to alleviate concerns about the > canaries? > > https://codereview.chromium.**org/14021003/<https://codereview.chromium.org/1... >
Message was sent while issue was closed.
Committed patchset #3 manually as r195325 (presubmit successful).
Message was sent while issue was closed.
On 2013/04/19 23:12:31, Dirk Pranke wrote: > Committed patchset #3 manually as r195325 (presubmit successful). FYI, Dirk talked to me before landing, I signed off. We need to get Blink unblocked here. We're going to watch the main waterfalls today. I pointed Dirk at the official waterfalls, too. If there's a problem we can revert this change.
Message was sent while issue was closed.
Please let me know when you think this CL has stuck so I can start building off it.
Message was sent while issue was closed.
On 2013/04/19 23:27:47, abarth wrote: > Please let me know when you think this CL has stuck so I can start building off > it. It didn't stick. The change was reverted in r195347. You win this round, Kerz :). |
