I know James has already commented on and pushed these (a bit
prematurely, he admits), but here are my comments.
On Nov 25, 2008, at 7:23 PM, Nigel Kersten wrote:
>
>
> Signed-off-by: Nigel Kersten <[EMAIL PROTECTED]>
> ---
> lib/puppet/provider/computer/computer.rb | 22 ++++
> lib/puppet/provider/group/directoryservice.rb | 5 +-
> .../provider/nameservice/directoryservice.rb | 109 +++++++++++
> ++++-----
> lib/puppet/type/computer.rb | 59 +++++++++++
> lib/puppet/type/group.rb | 38 ++++++--
> spec/unit/type/computer.rb | 86 +++++++++++
> ++++
> 6 files changed, 284 insertions(+), 35 deletions(-)
> create mode 100644 lib/puppet/provider/computer/computer.rb
> create mode 100644 lib/puppet/type/computer.rb
> create mode 100755 spec/unit/type/computer.rb
I think the group and computer code should have been split into two
commits -- they're basically orthogonal.
...skipping...
> - ds_value = dscl_plist[key][0] # only care about the
> first entry...
> + ds_value = dscl_plist[key]
> + if not @@ds_to_ns_attribute_map[ds_attribute]
> == :members # only members uses arrays so far
> + ds_value = ds_value[0]
> + end
Shouldn't this just be !=...?
> + newproperty(:ensure, :parent => Puppet::Property::Ensure) do
> + newvalue(:present) do
> + provider.create
> + end
> +
> + newvalue(:absent) do
> + Puppet.notice "prop ensure = absent"
> + provider.delete
> + end
> + end
I assume this is extra debugging lying around...
And you're missing docs for the :ensure property, and, of course, the
params below should have their docs extended a bit.
>
> + newparam(:name) do
> + desc "The "
> + isnamevar
> + end
> +
> + newparam(:realname) do
> + desc "realname"
> + end
> +
> + newproperty(:en_address) do
> + desc "The MAC address of the primary network interface.
> Must match en0."
> + end
> +
> + newproperty(:ip_address) do
> + desc "The IP Address of the Computer object."
> + end
> +
> +end
> \ No newline at end of file
> diff --git a/lib/puppet/type/group.rb b/lib/puppet/type/group.rb
> index 29486d3..1167962 100755
> --- a/lib/puppet/type/group.rb
> +++ b/lib/puppet/type/group.rb
> @@ -11,15 +11,22 @@ require 'facter'
>
> module Puppet
> newtype(:group) do
> - @doc = "Manage groups. This type can only create groups.
> Group
> - membership must be managed on individual users. This
> resource type
> - uses the prescribed native tools for creating groups
> and generally
> - uses POSIX APIs for retrieving information about them.
> It does
> - not directly modify ``/etc/group`` or anything.
> + @doc = "Manage groups. On most platforms this can only
> create groups.
> + Group membership must be managed on individual users.
> +
> + On OS X, group membership is managed as an attribute of
> the group.
> + This resource type uses the prescribed native tools for
> creating
> + groups and generally uses POSIX APIs for retrieving
> information
> + about them. It does not directly modify ``/etc/group``
> or anything.
>
> For most platforms, the tools used are ``groupadd`` and
> its ilk;
> - for Mac OS X, NetInfo is used. This is currently
> unconfigurable,
> - but if you desperately need it to be so, please contact
> us."
> + for Mac OS X, dscl/dseditgroup are used.
> +
> + This is currently unconfigurable, but if you
> desperately need it
> + to be so, please contact us."
This whole doc string is heinously out of date and should be
rewritten. It apparently hasn't been updated since providers were
developed.
I'd remove any mention of who does what, since the providers should
cover that.
If you need help rewriting it, let me know. :)
>
> + feature :manages_members,
> + "For directories where membership is an attribute of
> groups not users."
>
> ensurable do
> desc "Create or remove the group."
> @@ -73,13 +80,28 @@ module Puppet
> return gid
> end
> end
> +
> + newproperty(:members, :array_matching
> => :all, :required_features => :manages_members) do
> + desc "The members of the group. For directory services
> where group
> + membership is stored in the group objects, not the
> users."
Nice -- you've picked up a lot of the tricks. :)
I'll save commentary on the tests for the next version -- seems like
there'll be some refactoring anyway.
--
Susskind's Rule of Thumb:
Don't ask what they think. Ask what they do.
---------------------------------------------------------------------
Luke Kanies | http://reductivelabs.com | http://madstop.com
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Puppet Developers" group.
To post to this group, send email to puppet-dev@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/puppet-dev?hl=en
-~----------~----~----~----~------~----~------~--~---
