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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1232223003: http_cache: reorder states (Closed)

Created:
5 years, 5 months ago by hubbe
Modified:
5 years, 5 months ago
CC:
chromium-reviews, cbentzel+watch_chromium.org, gavinp+disk_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

http_cache: reorder states Reorder states in their "natural" order. Will be followed up with a CL to re-order the .cc file in the same way.

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+20 lines, -17 lines) Patch
M net/http/http_cache_transaction.h View 1 chunk +20 lines, -17 lines 0 comments Download

Messages

Total messages: 7 (1 generated)
hubbe
5 years, 5 months ago (2015-07-10 23:09:12 UTC) #2
rvargas (doing something else)
Sounds good.
5 years, 5 months ago (2015-07-11 02:20:54 UTC) #3
hubbe
On 2015/07/11 02:20:54, rvargas wrote: > Sounds good. Can I get an "lgtm" too?
5 years, 5 months ago (2015-07-11 22:43:19 UTC) #4
rvargas (doing something else)
On 2015/07/11 22:43:19, hubbe wrote: > On 2015/07/11 02:20:54, rvargas wrote: > > Sounds good. ...
5 years, 5 months ago (2015-07-13 18:22:26 UTC) #5
hubbe
On 2015/07/13 18:22:26, rvargas wrote: > On 2015/07/11 22:43:19, hubbe wrote: > > On 2015/07/11 ...
5 years, 5 months ago (2015-07-13 18:26:14 UTC) #6
rvargas (doing something else)
5 years, 5 months ago (2015-07-13 19:25:07 UTC) #7 
On 2015/07/13 18:26:14, hubbe wrote:
> On 2015/07/13 18:22:26, rvargas wrote:
> > On 2015/07/11 22:43:19, hubbe wrote:
> > > On 2015/07/11 02:20:54, rvargas wrote:
> > > > Sounds good.
> > > 
> > > Can I get an "lgtm" too?
> > 
> > But if this CL is just about reordering, the code should also be reordered
to
> > follow the new enum sequence.
> 
> I did that in a separate CL so that you could have a look at the proposed
order
> first.
> See https://codereview.chromium.org/1240503002/

Well, yeah, but I thought you were going to do that on this CL after we agreed
on the order.

Now that there's a CL for the rest, lgtm, but I think it's better to merge them
(you still need to change the header on the other CL).

(as a general rule, it's better if things are consistent after each refactoring
CL)

Powered by Google App Engine
This is Rietveld 408576698