Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2230423002:
Rename YouTube Flash embed experiment. (Closed)
|
Created:
4 years, 4 months ago by mlamouri (slow - plz ping) Modified:
4 years, 4 months ago CC:
chromium-reviews Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionRename YouTube Flash embed experiment.
This is fixing a couple of typos and style.
The CL originally disabled the feature too but a fix was found.
BUG=None
Committed: https://crrev.com/728ae20719724c5660c1eba10149cd4ac62b77e9
Cr-Commit-Position: refs/heads/master@{#411625}
Patch Set 1 #Patch Set 2 : plop #Patch Set 3 : fix other typo and attempt at keeping it enabled #Messages
Total messages: 28 (15 generated)
Description was changed from ========== Fix typo in experiment name. BUG=None ========== to ========== Rename YouTube Flash embed experiment and mark it disabled by default. There is a suspicious crash. It might be interesting to see what's happening when the feature is disabled by default. BUG=636226 ==========
mlamouri@chromium.org changed reviewers: + csharrison@chromium.org, thakis@chromium.org
PTAL
mlamouri@chromium.org changed reviewers: + jam@chromium.org - thakis@chromium.org
-thankis@ +jam@
LGTM, note you will have to change your finch config anyways to specify "enable-features".
On 2016/08/10 at 18:53:38, csharrison wrote: > LGTM, note you will have to change your finch config anyways to specify "enable-features". Yes, I'm aware of this :) Though, we should probably do that when we figure why there is a crash on Windows... :(
lgtm
Landing cause this is a huge crasher on canary.
The CQ bit was checked by csharrison@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: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
Looks like tests assume the feature is turned on. Let's land this tomorrow after amending this patch.
Description was changed from ========== Rename YouTube Flash embed experiment and mark it disabled by default. There is a suspicious crash. It might be interesting to see what's happening when the feature is disabled by default. BUG=636226 ========== to ========== Rename YouTube Flash embed experiment and mark it disabled by default. There is a crash because of this feature. The fix is still under review so we can land this in order to fix the crash while the fix is being taken care of. BUG=636226 ==========
The CQ bit was checked by mlamouri@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.
Description was changed from ========== Rename YouTube Flash embed experiment and mark it disabled by default. There is a crash because of this feature. The fix is still under review so we can land this in order to fix the crash while the fix is being taken care of. BUG=636226 ========== to ========== Rename YouTube Flash embed experiment. This is fixing a couple of typos and style. The CL originally disabled the feature too but a fix was found. BUG=None ==========
Description was changed from ========== Rename YouTube Flash embed experiment. This is fixing a couple of typos and style. The CL originally disabled the feature too but a fix was found. BUG=None ========== to ========== Rename YouTube Flash embed experiment. This is fixing a couple of typos and style. The CL originally disabled the feature too but a fix was found. BUG=None ==========
On 2016/08/11 at 00:12:32, csharrison wrote: > Looks like tests assume the feature is turned on. Let's land this tomorrow after amending this patch. I've landed a fix that should fix the crash. I will just land this as a rename to get ride of the typos.
The CQ bit was checked by mlamouri@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from jam@chromium.org, csharrison@chromium.org Link to the patchset: https://codereview.chromium.org/2230423002/#ps40001 (title: "fix other typo and attempt at keeping it enabled")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Rename YouTube Flash embed experiment. This is fixing a couple of typos and style. The CL originally disabled the feature too but a fix was found. BUG=None ========== to ========== Rename YouTube Flash embed experiment. This is fixing a couple of typos and style. The CL originally disabled the feature too but a fix was found. BUG=None Committed: https://crrev.com/728ae20719724c5660c1eba10149cd4ac62b77e9 Cr-Commit-Position: refs/heads/master@{#411625} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/728ae20719724c5660c1eba10149cd4ac62b77e9 Cr-Commit-Position: refs/heads/master@{#411625} |
