|
|
Created:
4 years, 2 months ago by maksims (do not use this acc) Modified:
4 years, 2 months ago CC:
chromium-reviews, cbentzel+watch_chromium.org, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org, abarth-chromium, Aaron Boodman, darin (slow to review) Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionMerge ProxyResolverMojo::Job and ProxyResolverMojo::RequestImpl
This is a follow up CL, which merges RequestImpl and Job
together.
Prev CL is here - https://chromiumcodereview.appspot.com/2299963002/
BUG=478934
Committed: https://crrev.com/f1aa7d4e5ec785cc01875b4eecf46844161d6d3c
Cr-Commit-Position: refs/heads/master@{#426862}
Patch Set 1 #
Total comments: 1
Patch Set 2 : nits #Messages
Total messages: 19 (14 generated)
Description was changed from ========== remove requestimpl BUG= ========== to ========== Merge ProxyResolverMojo::Job and ProxyResolverMojo::RequestImpl This is a follow up CL, which merges RequestImpl and Job together. Prev CL is here - https://chromiumcodereview.appspot.com/2299963002/ BUG=478934 ==========
maksim.sisov@intel.com changed reviewers: + eroman@chromium.org, sammc@chromium.org
The CQ bit was checked by maksim.sisov@intel.com 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...
as per our discussion, here is the follow up.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm https://codereview.chromium.org/2430953004/diff/1/net/proxy/proxy_resolver_fa... File net/proxy/proxy_resolver_factory_mojo.cc (right): https://codereview.chromium.org/2430953004/diff/1/net/proxy/proxy_resolver_fa... net/proxy/proxy_resolver_factory_mojo.cc:277: *request = std::move(job); How about combining these two lines into one: *request = base::MakeUnique<Job>(this, url, results, callback, net_log);
The CQ bit was checked by maksim.sisov@intel.com 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.
The CQ bit was checked by maksim.sisov@intel.com
The patchset sent to the CQ was uploaded after l-g-t-m from eroman@chromium.org Link to the patchset: https://codereview.chromium.org/2430953004/#ps20001 (title: "nits")
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.
Description was changed from ========== Merge ProxyResolverMojo::Job and ProxyResolverMojo::RequestImpl This is a follow up CL, which merges RequestImpl and Job together. Prev CL is here - https://chromiumcodereview.appspot.com/2299963002/ BUG=478934 ========== to ========== Merge ProxyResolverMojo::Job and ProxyResolverMojo::RequestImpl This is a follow up CL, which merges RequestImpl and Job together. Prev CL is here - https://chromiumcodereview.appspot.com/2299963002/ BUG=478934 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Merge ProxyResolverMojo::Job and ProxyResolverMojo::RequestImpl This is a follow up CL, which merges RequestImpl and Job together. Prev CL is here - https://chromiumcodereview.appspot.com/2299963002/ BUG=478934 ========== to ========== Merge ProxyResolverMojo::Job and ProxyResolverMojo::RequestImpl This is a follow up CL, which merges RequestImpl and Job together. Prev CL is here - https://chromiumcodereview.appspot.com/2299963002/ BUG=478934 Committed: https://crrev.com/f1aa7d4e5ec785cc01875b4eecf46844161d6d3c Cr-Commit-Position: refs/heads/master@{#426862} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/f1aa7d4e5ec785cc01875b4eecf46844161d6d3c Cr-Commit-Position: refs/heads/master@{#426862} |