Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(225)

Issue 1574423003: Revert of Add a whitelist for QUIC hosts. (Closed)

Created:
4 years, 11 months ago by trchen
Modified:
4 years, 11 months ago
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of Add a whitelist for QUIC hosts. (patchset #7 id:140001 of https://codereview.chromium.org/1580583002/ ) Reason for revert: Speculative revert for Android Cronet bots failure (CronetTestInstrumentation) Original issue's description: > Add a whitelist for QUIC hosts. > > BUG=576402 > > Committed: https://crrev.com/1ad82bbc44d87669926c9cffe2ba77564df420a3 > Cr-Commit-Position: refs/heads/master@{#368810} TBR=agl@chromium.org,rsleevi@chromium.org,rch@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=576402 Committed: https://crrev.com/d940c626454dcf80efdcff9c40dd99ba055fb2b2 Cr-Commit-Position: refs/heads/master@{#369050}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+0 lines, -116 lines) Patch
M chrome/browser/io_thread.h View 3 chunks +0 lines, -7 lines 0 comments Download
M chrome/browser/io_thread.cc View 3 chunks +0 lines, -22 lines 0 comments Download
M chrome/browser/io_thread_unittest.cc View 2 chunks +0 lines, -27 lines 0 comments Download
M chrome/common/chrome_switches.h View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/common/chrome_switches.cc View 1 chunk +0 lines, -3 lines 0 comments Download
M net/http/http_network_session.h View 2 chunks +0 lines, -4 lines 0 comments Download
M net/http/http_stream_factory_impl.h View 1 chunk +0 lines, -4 lines 0 comments Download
M net/http/http_stream_factory_impl.cc View 3 chunks +0 lines, -17 lines 0 comments Download
M net/http/transport_security_state.h View 1 chunk +0 lines, -6 lines 0 comments Download
M net/http/transport_security_state.cc View 1 chunk +0 lines, -19 lines 0 comments Download
M net/quic/quic_network_transaction_unittest.cc View 1 chunk +0 lines, -6 lines 0 comments Download

Messages

Total messages: 8 (1 generated)
trchen
Created Revert of Add a whitelist for QUIC hosts.
4 years, 11 months ago (2016-01-13 00:23:33 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1574423003/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1574423003/1
4 years, 11 months ago (2016-01-13 00:24:39 UTC) #2
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 11 months ago (2016-01-13 00:28:03 UTC) #3
commit-bot: I haz the power
Patchset 1 (id:??) landed as https://crrev.com/d940c626454dcf80efdcff9c40dd99ba055fb2b2 Cr-Commit-Position: refs/heads/master@{#369050}
4 years, 11 months ago (2016-01-13 00:29:22 UTC) #5
Ryan Sleevi
Not LGTM. We should not be reverting for FYI bots.
4 years, 11 months ago (2016-01-13 00:35:29 UTC) #6
trchen
On 2016/01/13 00:35:29, Ryan Sleevi wrote: > Not LGTM. > > We should not be ...
4 years, 11 months ago (2016-01-13 00:47:26 UTC) #7
Ryan Sleevi
4 years, 11 months ago (2016-01-13 00:53:10 UTC) #8
Message was sent while issue was closed.
On 2016/01/13 00:47:26, trchen wrote:
 
> What is your recommended way to proceed? The instruction on
> https://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium
> suggests "In all cases, the preferred way of dealing with alerts is to revert
> the offending patch, even if the failure does not close the tree.".
> 
> Let me know what is the better practice and I will do that next time. Thanks!

If it's an FYI bot (e.g. no trybot, not on the CQ, not reproducible by any
developer), then you handle it asynchronously

From https://www.chromium.org/developers/tree-sheriffs
"For bots that are not covered by the Commit Queue and if the author is online,
it's fine to ask them to fix asynchronously (since it shouldn't be blocking
anyone, and it's not the author's fault as the change landed through the CQ)."

Basically, if it's not on the CQ, it's strongly discouraged from reverting, and
the bot owner(s) / sheriffs are the ones that should drive the investigation and
green-ing the bot.

Powered by Google App Engine
This is Rietveld 408576698