Skip to content
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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

xepa
Copy link

@xepa xepa commented Sep 12, 2023

replaced is_domain_name and all validate_* with more modern versions

Fixes #76


This change is Reviewable

@maxadamo
Copy link

maxadamo commented Oct 17, 2023

@xepa could you please fix this one as well:
https://github.com/kakwa/puppet-samba/blob/master/manifests/log.pp#L14

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?

Copy link

@maxadamo maxadamo left a 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: :shipit: complete! all files reviewed, all discussions resolved (waiting on @xepa)

@xepa
Copy link
Author

xepa commented Oct 26, 2023

thank you @maxadamo for pointing out the missing is_hash fix

@xepa
Copy link
Author

xepa commented Jan 29, 2024

Is there anything that I can do to expedite a merge ?

@maxadamo
Copy link

@kakwa are willing to review this change?

@xepa
Copy link
Author

xepa commented Mar 13, 2024

@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.

@bastelfreak
Copy link
Contributor

Hi,
I've no merge powers here.



unless is_domain_name($realm){
) inherits samba::params {
Copy link
Contributor

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.

@maxadamo
Copy link

maxadamo commented Mar 13, 2024

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?
The module uses old fashioned language style now.

@bastelfreak
Copy link
Contributor

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.

@kakwa
Copy link
Owner

kakwa commented Mar 13, 2024

@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 Kakwa, already joined the IRC channel.

edit: did the etymology, I love your project name.

@xepa
Copy link
Author

xepa commented Jul 4, 2024

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)

@iainhallam
Copy link

@kakwa @maxadamo @bastelfreak Did anything come of moving this module to Vox Populi?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

updates for stdlib 9.x.x
5 participants