Refactoring client/version to util

Refactoring client/version to util

BUG=622374
Committed: https://crrev.com/5bba9e15186207d6e88aa1c43ee17230a6578c25
Cr-Commit-Position: refs/heads/master@{#403201}

[RyanSturm, 2016-06-28 22:08:35]

tbansal: PTAL

[RyanSturm, 2016-06-28 22:38:24]

https://codereview.chromium.org/2104143002/diff/1/components/data_reduction_p...
File components/data_reduction_proxy/core/common/data_reduction_proxy_util.cc
(right):

https://codereview.chromium.org/2104143002/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/common/data_reduction_proxy_util.cc:31: //
Returns the version of Chromium that is being used, e.g. "1.2.3.4".
Move comment to header.

https://codereview.chromium.org/2104143002/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/common/data_reduction_proxy_util.cc:48: //
Returns the build and patch numbers of |version_string|. |version_string|
Move comment to header.

https://codereview.chromium.org/2104143002/diff/1/components/data_reduction_p...
File components/data_reduction_proxy/core/common/data_reduction_proxy_util.h
(right):

https://codereview.chromium.org/2104143002/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/common/data_reduction_proxy_util.h:55:
const char* GetString(Client client);
Add comment

[RyanSturm, 2016-06-28 22:53:30]

On 2016/06/28 22:38:24, RyanSturm wrote:
>
https://codereview.chromium.org/2104143002/diff/1/components/data_reduction_p...
> File components/data_reduction_proxy/core/common/data_reduction_proxy_util.cc
> (right):
> 
>
https://codereview.chromium.org/2104143002/diff/1/components/data_reduction_p...
> components/data_reduction_proxy/core/common/data_reduction_proxy_util.cc:31:
//
> Returns the version of Chromium that is being used, e.g. "1.2.3.4".
> Move comment to header.
> 
>
https://codereview.chromium.org/2104143002/diff/1/components/data_reduction_p...
> components/data_reduction_proxy/core/common/data_reduction_proxy_util.cc:48:
//
> Returns the build and patch numbers of |version_string|. |version_string|
> Move comment to header.
> 
>
https://codereview.chromium.org/2104143002/diff/1/components/data_reduction_p...
> File components/data_reduction_proxy/core/common/data_reduction_proxy_util.h
> (right):
> 
>
https://codereview.chromium.org/2104143002/diff/1/components/data_reduction_p...
> components/data_reduction_proxy/core/common/data_reduction_proxy_util.h:55:
> const char* GetString(Client client);
> Add comment

All Done.

[RyanSturm, 2016-06-29 17:55:11]

ryansturm@chromium.org changed reviewers:
+ tbansal@chromium.org

[RyanSturm, 2016-06-29 17:55:11]

tbansal: PTAL

[tbansal1, 2016-06-29 17:57:53]

https://codereview.chromium.org/2104143002/diff/20001/components/data_reducti...
File components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.h
(right):

https://codereview.chromium.org/2104143002/diff/20001/components/data_reducti...
components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.h:23:
#include
"components/data_reduction_proxy/core/common/data_reduction_proxy_util.h"
why is this included here?

[RyanSturm, 2016-06-29 17:58:48]

On 2016/06/29 17:57:53, tbansal1 wrote:
>
https://codereview.chromium.org/2104143002/diff/20001/components/data_reducti...
> File
components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.h
> (right):
> 
>
https://codereview.chromium.org/2104143002/diff/20001/components/data_reducti...
>
components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.h:23:
> #include
> "components/data_reduction_proxy/core/common/data_reduction_proxy_util.h"
> why is this included here?

Client is typedef'd in util.h (and a member of DRPIOData)

[RyanSturm, 2016-06-29 19:01:40]

https://codereview.chromium.org/2104143002/diff/20001/components/data_reducti...
File components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.h
(right):

https://codereview.chromium.org/2104143002/diff/20001/components/data_reducti...
components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.h:23:
#include
"components/data_reduction_proxy/core/common/data_reduction_proxy_util.h"
On 2016/06/29 17:57:53, tbansal1 wrote:
> why is this included here?

Acknowledged.

[tbansal1, 2016-06-29 23:21:49]

https://codereview.chromium.org/2104143002/diff/40001/components/data_reducti...
File components/data_reduction_proxy/core/common/data_reduction_proxy_util.h
(right):

https://codereview.chromium.org/2104143002/diff/40001/components/data_reducti...
components/data_reduction_proxy/core/common/data_reduction_proxy_util.h:28:
typedef enum {
can this be changed to enum class to tighten the scope?

https://codereview.chromium.org/2104143002/diff/40001/components/data_reducti...
components/data_reduction_proxy/core/common/data_reduction_proxy_util.h:56:
const char* GetString(Client client);
nit: s/GetString/GetStringForClient/

[RyanSturm, 2016-06-29 23:37:12]

https://codereview.chromium.org/2104143002/diff/40001/components/data_reducti...
File components/data_reduction_proxy/core/common/data_reduction_proxy_util.h
(right):

https://codereview.chromium.org/2104143002/diff/40001/components/data_reducti...
components/data_reduction_proxy/core/common/data_reduction_proxy_util.h:28:
typedef enum {
On 2016/06/29 23:21:49, tbansal1 wrote:
> can this be changed to enum class to tighten the scope?

Done.

https://codereview.chromium.org/2104143002/diff/40001/components/data_reducti...
components/data_reduction_proxy/core/common/data_reduction_proxy_util.h:56:
const char* GetString(Client client);
On 2016/06/29 23:21:49, tbansal1 wrote:
> nit: s/GetString/GetStringForClient/

Done.

[tbansal1, 2016-06-29 23:45:37]

Thanks. This is much cleaner, and easier to understand.
lgtm % 2 nits:
1. Add 622374 as the bug in the CL description
2. Make sure to run the cronet testbot before landing the CL.

[RyanSturm, 2016-06-29 23:49:03]

Description was changed from

==========
Refactoring client/version to util

BUG=
==========

to

==========
Refactoring client/version to util

BUG=622374
==========

[RyanSturm, 2016-06-30 17:02:43]

The CQ bit was checked by ryansturm@chromium.org

[commit-bot: I haz the power, 2016-06-30 17:03:08]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-30 17:07:52]

Description was changed from

==========
Refactoring client/version to util

BUG=622374
==========

to

==========
Refactoring client/version to util

BUG=622374
==========

[commit-bot: I haz the power, 2016-06-30 17:07:53]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-06-30 17:07:55]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-06-30 17:09:50]

Description was changed from

==========
Refactoring client/version to util

BUG=622374
==========

to

==========
Refactoring client/version to util

BUG=622374

Committed: https://crrev.com/5bba9e15186207d6e88aa1c43ee17230a6578c25
Cr-Commit-Position: refs/heads/master@{#403201}
==========

[commit-bot: I haz the power, 2016-06-30 17:09:51]

Patchset 4 (id:??) landed as
https://crrev.com/5bba9e15186207d6e88aa1c43ee17230a6578c25
Cr-Commit-Position: refs/heads/master@{#403201}