-
Notifications
You must be signed in to change notification settings - Fork 52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
replaced deprecated stdlib functions #77
base: master
Are you sure you want to change the base?
Conversation
@xepa could you please fix this one as well: it should become: unless $sambaclassloglevel =~ Hash @kakwa would you mind reviewing this commit, also considering the change that I am proposing, and creating a new release? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
more changes needed:
https://github.com/kakwa/puppet-samba/blob/master/manifests/log.pp#L14
unless $sambaclassloglevel =~ Hash
Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @xepa)
thank you @maxadamo for pointing out the missing is_hash fix |
Is there anything that I can do to expedite a merge ? |
@kakwa are willing to review this change? |
@kakwa would it be possible that you can take a look at this PR and merge / tag ? I will CC @bastelfreak (as contributor) maybe he can take on this request. |
Hi, |
|
||
|
||
unless is_domain_name($realm){ | ||
) inherits samba::params { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my recommendation is to add the datatypes to the class parameters,that makes the whole file shorter and enabled puppet-strings to pick up the types for the automatic documentation.
agree with the above comment. But, unless any objection from @kakwa , we may want to fork this module. @bastelfreak do we have Samba in voxPupuli? |
Vox Pupuli doesn't have a samba module. I suggest you hop into our IRC or Slack channel and ask around. I think there are some more people that use a samba module, but I don't know which one. |
@maxadamo @bastelfreak I will check on IRC/Slack and discuss the possibility of transferring full ownership of this module. I'm completely ok with a fork btw, but a full ownership transfer would be preferable IMHO, including the Puppet Forge Stuff if still relevant. I will try to ping you on IRC in the next few days. Nick edit: did the etymology, I love your project name. |
Is there something that I can do to expedite the merger of this PR ? (it has been a while now .. and I would really like to switch back to the official version instead of my branch) |
@kakwa @maxadamo @bastelfreak Did anything come of moving this module to Vox Populi? |
replaced
is_domain_name
and allvalidate_*
with more modern versionsFixes #76
This change is