Also, I'll add more documentation in policy_templates.json to make sure people understand what default_for_enterprise_users means.

Alternatively, we could get rid of it completely if that's feasible :)

I've reviewed two CLs so far which proposed to use default_for_enterprise_users:
The first one actually wanted to document what the default is.
The second one wanted to use it with device policy, which does not work (and it's only hinted by the fact that it's called .*_users, but not mentioned anywhere explicitly).

+1 for improving the documentation. 

I was just wondering why this doesn't work for device policy, but it's because user policy uses autogen'ed code and our device policy is fully manual. I wish we'd switch to autogen'ed code for device policy as well.

Also, the generated function is only invoked in user policy code.
Also, device policies tend to be evaluated directly from the proto, i.e.
proto -> PolicyMap (device_policy_decoder)
proto -> CrosSettings (device_settings_provider) -> logic

So even if we invoked the function, device_settings_provider would not care.

Am 07.02.2018 10:22 schrieb "ljusten via monorail" <
monorail+v2.433221273@chromium.org>:

Julian suggested that it's still a good thing to have something like this for device policy for documentation.

We could keep default_for_enterprise_users which can only be used with user policy and actually sets a default.
Plus we could add default_for_enrolled_devices_doc, which would be documented clearly as only documenting the default and not doing anything implicitly.

WDYT?

I also find it confusing that default_for_enterprise_users doesn't have any effect on device policies for which default is defined by device_policy_decoder logic.
For 'OffHours' feature we were relying on empty proto values for the policy as "default" and the expected "default" implementation for missing proto value is handled in code (device_policy_decoder).

However, there should be some documentation about this "default" logic.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/da51f80f0a557b38ffa296da8162326bdd3a3b58

commit da51f80f0a557b38ffa296da8162326bdd3a3b58
Author: Pavol Marko <pmarko@chromium.org>
Date: Tue Mar 13 10:43:25 2018

Add PRESUBMIT warning about default_for_enteprise_users with device policies

Add a PRESUBMIT check for policy_templates.json which generates an error
when the default_for_enterprise_users flag is used with a policy that is
marked 'device_only': True.
Also introduce doc_default_for_managed_devices to be able to document a
(manually implemented) special default value for device policies on
managed devices.
There's also a PRESUBMIT checking that doc_default_for_managed_devices
is not used with a 'device_only': False policy.

Bug: 809653
Change-Id: I06a9a7a23998d918b252ad3718a279804d4adaa8
Reviewed-on: https://chromium-review.googlesource.com/926324
Reviewed-by: Lutz Justen <ljusten@chromium.org>
Reviewed-by: Drew Wilson <atwilson@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Sergey Poromov <poromov@chromium.org>
Commit-Queue: Pavol Marko <pmarko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542764}
[modify] https://crrev.com/da51f80f0a557b38ffa296da8162326bdd3a3b58/components/policy/resources/policy_templates.json
[modify] https://crrev.com/da51f80f0a557b38ffa296da8162326bdd3a3b58/components/policy/tools/syntax_check_policy_template_json.py