Hide Night Light behind a flag

Hide Night Light behind a flag

Feature is not ready to ship yet in M60. This CL hides it behind a flag
to be merged back to M60.

BUG=729226
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2921973002
Cr-Commit-Position: refs/heads/master@{#477054}
Committed: https://chromium.googlesource.com/chromium/src/+/9e0a18b3e858c685893f1b9c2c59646fa555f187

[afakhry, 2017-06-02 23:20:08]

Description was changed from

==========
Hide Night Light behind a flag

Feature is not ready to ship yet in M60. This CL hides it behind a flag
to be merged back to M60.

BUG=729226
==========

to

==========
Hide Night Light behind a flag

Feature is not ready to ship yet in M60. This CL hides it behind a flag
to be merged back to M60.

BUG=729226
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[afakhry, 2017-06-02 23:20:15]

The CQ bit was checked by afakhry@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-02 23:20:40]

Dry run: CQ is trying da patch.

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

[afakhry, 2017-06-02 23:31:25]

Patchset #1 (id:1) has been deleted

[afakhry, 2017-06-02 23:31:33]

The CQ bit was checked by afakhry@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-02 23:32:20]

Dry run: CQ is trying da patch.

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

[afakhry, 2017-06-02 23:33:27]

afakhry@chromium.org changed reviewers:
+ jamescook@chromium.org, stevenjb@chromium.org

[afakhry, 2017-06-02 23:33:28]

jamescook@ Please take a look at ash.

stevenjb@ Please take a look at settings.

Thanks!

[James Cook, 2017-06-02 23:40:05]

ash LGTM with nits

https://codereview.chromium.org/2921973002/diff/20001/ash/shell.h
File ash/shell.h (right):

https://codereview.chromium.org/2921973002/diff/20001/ash/shell.h#newcode341
ash/shell.h:341: DCHECK(NightLightController::IsFeatureEnabled());
If you want to do this I would move the implementation into the .cc file and not
include the header. (It's OK to have hacker_style() methods with the impl in the
.cc as long as the impl is cheap.)

https://codereview.chromium.org/2921973002/diff/20001/ash/system/night_light/...
File ash/system/night_light/night_light_controller.cc (right):

https://codereview.chromium.org/2921973002/diff/20001/ash/system/night_light/...
ash/system/night_light/night_light_controller.cc:69:
base::CommandLine::ForCurrentProcess()->AppendSwitch(
nit: I think it would be better to do this in the unit test files, even if you
had to do it in more than one place.

[afakhry, 2017-06-03 00:08:51]

The CQ bit was checked by afakhry@chromium.org to run a CQ dry run

[afakhry, 2017-06-03 00:09:03]

https://codereview.chromium.org/2921973002/diff/20001/ash/shell.h
File ash/shell.h (right):

https://codereview.chromium.org/2921973002/diff/20001/ash/shell.h#newcode341
ash/shell.h:341: DCHECK(NightLightController::IsFeatureEnabled());
On 2017/06/02 23:40:05, James Cook wrote:
> If you want to do this I would move the implementation into the .cc file and
not
> include the header. (It's OK to have hacker_style() methods with the impl in
the
> .cc as long as the impl is cheap.)

Done.

https://codereview.chromium.org/2921973002/diff/20001/ash/system/night_light/...
File ash/system/night_light/night_light_controller.cc (right):

https://codereview.chromium.org/2921973002/diff/20001/ash/system/night_light/...
ash/system/night_light/night_light_controller.cc:69:
base::CommandLine::ForCurrentProcess()->AppendSwitch(
On 2017/06/02 23:40:05, James Cook wrote:
> nit: I think it would be better to do this in the unit test files, even if you
> had to do it in more than one place.

Done.

[commit-bot: I haz the power, 2017-06-03 00:09:03]

Dry run: 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, 2017-06-03 02:56:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-03 02:56:58]

Dry run: This issue passed the CQ dry run.

[stevenjb, 2017-06-05 17:19:26]

lgtm

[afakhry, 2017-06-05 17:40:12]

The CQ bit was checked by afakhry@chromium.org

[afakhry, 2017-06-05 17:40:13]

The patchset sent to the CQ was uploaded after l-g-t-m from
jamescook@chromium.org
Link to the patchset: https://codereview.chromium.org/2921973002/#ps40001
(title: "James' comments")

[commit-bot: I haz the power, 2017-06-05 17:40:38]

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, 2017-06-05 19:23:18]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1496684412991500,
"parent_rev": "123a65b6261c9aedc86ac67e5f270f9a9aa917bd", "commit_rev":
"9e0a18b3e858c685893f1b9c2c59646fa555f187"}

[commit-bot: I haz the power, 2017-06-05 19:23:29]

Description was changed from

==========
Hide Night Light behind a flag

Feature is not ready to ship yet in M60. This CL hides it behind a flag
to be merged back to M60.

BUG=729226
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Hide Night Light behind a flag

Feature is not ready to ship yet in M60. This CL hides it behind a flag
to be merged back to M60.

BUG=729226
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2921973002
Cr-Commit-Position: refs/heads/master@{#477054}
Committed:
https://chromium.googlesource.com/chromium/src/+/9e0a18b3e858c685893f1b9c2c59...
==========

[commit-bot: I haz the power, 2017-06-05 19:23:31]

Committed patchset #2 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/9e0a18b3e858c685893f1b9c2c59...