add suffix() to go along with prefix() #138
Conversation
| suffix = arguments[1] if arguments[1] | ||
|
|
||
| if suffix | ||
| unless suffix.is_a?(String) |
There was a problem hiding this comment.
It would be good to have a test to ensure that the suffix method fails if the second argument is a string. Also, what happens if the suffixed value is an integer?
There was a problem hiding this comment.
This "suffix" function is a carbon copy of stdlib's existing "prefix" function, with the only change being to append rather than prepend (which is a single line change, aside from s/prefix/suffix/g).
So I'm not sure how to respond to these comments, since they are actually comments about the already-shipping "prefix" function, authored by "Krzysztof Wilczynski" (perhaps on github somewhere?).
Given that "prefix" is considered good as-is, accepting 'suffix' and then filing an issue against puppetlabs-stdlib to address these comments for both prefix and suffix would ensure that your comments are addressed without unnecessarily delaying inclusion of a useful function.
(This pull request was submitted at the urging of our trainer, so you can also come find me in the PDX classroom today and tomorrow between 9a and 4p, if that's useful.)
|
@floatingatoll it looks like we don't have a CLA on file for you; would you be willing to sign it at https://cla.puppetlabs.com ? |
|
done On Wed, Mar 27, 2013 at 12:56 PM, Adrien Thebo [email protected]:
|
|
CLA Signed by floatingatoll on 2013-03-27 13:02:33 -0700 |
|
Merged into master as f0db049. Thanks again for the contribution! -Adrien |
No description provided.