-
Notifications
You must be signed in to change notification settings - Fork 580
Set arity on all functions #249
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
Conversation
Should this be against 4.x instead of master? |
Take 3, now that PE 2.x is also deprecated |
c919bf8
to
60743f6
Compare
👍 |
Note that this removes the testing on Puppet 2.x as that is deprecated and will fail the arity tests. |
newfunction(:join, :type => :rvalue, :doc => <<-EOS | ||
This function joins an array into a string using a separator. | ||
newfunction(:join, :type => :rvalue, :arity => -2, :doc => <<-EOS | ||
This function joins an array into a string using a seperator. |
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.
i think you corrected this word wrong… seperator → separator
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.
Fixed. Don't know if it was the rebase that undid some previous spelling fix maybe.
except for the "typo" correction, 👍 |
Use the function arity feature to remove a lot of boilerplate code from the functions.
Someone should give this guy a trophy... |
hehe, I believe that already happened :D |
👑 (it's a pity this isn't a tiara…) |
🏆 |
@igalic @jeffmccune -- are there still backwards compatiblity concerns about this PR? I think a determination needs to be made so policy can be set with respect to new functions. If this PR is going in, I think the policy should be no new functions will be merged without arty declared (I commented about this on #259). |
yesno. we talked about this thing today in the modules triage, and we'll be sending out an email to dev & users@ with our prophecies. |
Now this has merge conflicts (again). Could you please poke me if you want to merge this so I don't have to fix those conflicts all the time. |
Given that the puppet 4 function api dispatches take care of arity and puppet 3 will eventually be EOL'd, I'm going to just close this... when we drop puppet 3 we'll convert all functions to the 4 api. |
Use the function arity feature to remove a lot of boilerplate code from
the functions.
take 2, now that puppet 2.x support is deprecated (again kinda)