Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2524733008:
Fix for google:instantExtendedEnabledKey replacement (Closed)
|
Created:
4 years ago by Alexander Yashkin Modified:
4 years ago CC:
chromium-reviews Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionFix for google:instantExtendedEnabledKey replacement
This is fix for minor bug that was inserted in
google:instantExtendedEnabledKey replacement in
https://codereview.chromium.org/2516963002/.
Incorrect string literal was used in replacement.
R=treib@chromium.org, pkasting@chromium.org
Committed: https://crrev.com/e128f9d8f6ebe7064709a42805e3d3233f521e31
Cr-Commit-Position: refs/heads/master@{#434456}
Patch Set 1 #Patch Set 2 : Added unittest #
Messages
Total messages: 27 (16 generated)
The CQ bit was checked by a-v-y@yandex-team.ru 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: No L-G-T-M from a valid reviewer yet. CQ run can only be started by full committers or once the patch has received an L-G-T-M from a full committer. Even if an L-G-T-M may have been provided, it was from a non-committer, _not_ a full super star committer. See http://www.chromium.org/getting-involved/become-a-committer Note that this has nothing to do with OWNERS files.
The CQ bit was checked by treib@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...
LGTM, thanks! (but I don't own this code)
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: 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_...)
Honestly I can't conceive mentally of which thing should be used when. I'm going to assume you checked carefully and did the right thing in each place. Can there be a test that would have caught this bug, so we don't make the same mistake again later? LGTM otherwise.
On 2016/11/23 at 17:40:33, pkasting wrote: > Honestly I can't conceive mentally of which thing should be used when. I'm going to assume you checked carefully and did the right thing in each place. > > Can there be a test that would have caught this bug, so we don't make the same mistake again later? > > LGTM otherwise. Added unittest, but still after searching through code my impressions are that this functionality was either not implemented as intended or is already deleted. Especially after seeing treib efforts in "Remove dead search/instant code" https://bugs.chromium.org/p/chromium/issues/detail?id=627747.
The CQ bit was checked by a-v-y@yandex-team.ru
The patchset sent to the CQ was uploaded after l-g-t-m from treib@chromium.org, pkasting@chromium.org Link to the patchset: https://codereview.chromium.org/2524733008/#ps20001 (title: "Added unittest")
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: cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromeos_amd64-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromeos_x86-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by a-v-y@yandex-team.ru
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: cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromeos_x86-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by a-v-y@yandex-team.ru
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": 1480060476569580,
"parent_rev": "a48db575b93c1b2004901ca3673492def7b8d7fe", "commit_rev":
"15ee861d59b141c247cb3c599dc9490aa0834edd"}
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Fix for google:instantExtendedEnabledKey replacement This is fix for minor bug that was inserted in google:instantExtendedEnabledKey replacement in https://codereview.chromium.org/2516963002/. Incorrect string literal was used in replacement. R=treib@chromium.org, pkasting@chromium.org ========== to ========== Fix for google:instantExtendedEnabledKey replacement This is fix for minor bug that was inserted in google:instantExtendedEnabledKey replacement in https://codereview.chromium.org/2516963002/. Incorrect string literal was used in replacement. R=treib@chromium.org, pkasting@chromium.org Committed: https://crrev.com/e128f9d8f6ebe7064709a42805e3d3233f521e31 Cr-Commit-Position: refs/heads/master@{#434456} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/e128f9d8f6ebe7064709a42805e3d3233f521e31 Cr-Commit-Position: refs/heads/master@{#434456}
Message was sent while issue was closed.
A revert of this CL (patchset #2 id:20001) has been created in https://codereview.chromium.org/2656893002/ by pkasting@chromium.org. The reason for reverting is: Part 1 of 2: Reverting change that caused bug 680197. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
