dayglojesus / managedmac

Comprehensive Puppet module for OS X.
http://dayglojesus.github.io/managedmac/
Apache License 2.0
62 stars 21 forks source link

add 'system.device.dvd.setregion.initial' settings to authorization.pp #11

Closed groob closed 10 years ago

groob commented 10 years ago

Allows standard users to set the DVD region code when prompted.

dayglojesus commented 10 years ago

@groob thanks for your patch and the testing you are doing, keep it going! I do appreciate it!

I will review the PR as soon as I can. Again, my thanks!

dayglojesus commented 10 years ago

@groob there are a few cascading changes that need to be made with this commit:

  1. spec tests for the class need to be updated
  2. class documentation is out of date (already needed to be fixed)
  3. sample.yaml needs to have the listed params updated

Do you plan on adding any other authorization.pp changes? If not, I will get this done today. If you do, then I will wait until you think you have the required features.

Please let me know.

groob commented 10 years ago

I'm done adding changes to authorization.pp

dayglojesus commented 10 years ago

@groob I've merged your PR into 0.4.3. It won't make it to the master branch until 0.4.4 when I release the new module to the Forge tomorrow.

I will close this now. Thanks again for the patch! Cheers.

dayglojesus commented 10 years ago

@groob just a heads up here... 0.4.5 is released, built and on the Puppet forge.

NOTE: I have made some modifications to the authorization class.

  1. I changed the way system.device.dvd.setregion.initial works, rather than changing the 'group' attrib, it now mods the 'auth_class' https://github.com/dayglojesus/managedmac/blob/master/manifests/authorization.pp#L153
  2. I modified a comment to match the Apple defaults exactly so that it doesn't trigger unnecessary changes. The comment you added had one less space character. It's Apple's fault, not yours, but that's the way it is set, so we should keep it that way. https://github.com/dayglojesus/managedmac/blob/master/manifests/authorization.pp#L157
  3. I changed the PARAM NAME to delineate it from the pref panes params; it's now called $allow_dvd_setregion_initial

Hope this doesn't throw things too out of whack for you. Sorry!