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

(docs) update documentation wrt functions moved to puppet #922

Conversation

hlindberg
Copy link
Contributor

No description provided.

Since PUP-5556, the regular sprintf function in puppet core supports
named fields from a hash.

The sprintf_hash is therefore not needed.
The documentation did not include that empty-string are not counted.
It also stated that a "match" was made with the second argument when it
is an equals operation.
The delete() function can be performed with the - operator, with
regsubst (on string) or with the filter function. All of which are more
flexible than the stdlib function.

This adds examples of their use.
This corrects a faulty example in getparam() (it would not show what was
intended because of order of evaluation).
The example is now correct.

A note is also added that shows the equivalent operation in the
puppet language.
This adds a note that non existing variables result in an undef value
being returned.
Before this, only the strftime.rb impl of the funciton noted
that it was replaced with built-in equiv in Puppet 4.8.0.

This updates the README, the unit and acceptance test.
@pmcmaw
Copy link
Contributor

pmcmaw commented Jun 18, 2018

Ran through our internal adhoc pipeline with no issues and all changes look as expected.
Happy enough when @clairecadman has checked the docs to merge.

@hlindberg
Copy link
Contributor Author

@pmcmaw I just pushed one more commit (after your check). No more commits coming now - promise... (unless someone wants something changed).

@pmcmaw
Copy link
Contributor

pmcmaw commented Jun 19, 2018

So running this through the adhoc pipeline doe not resolve the issue. After some high level looking it seems to be continuing to run the test. I am carrying out some investigation into this.

@pmcmaw
Copy link
Contributor

pmcmaw commented Jun 19, 2018

Hey @hlindberg ,

So after investigation and confirmation on our internal adhoc pipelines would it be possible to replace Puppet.version with return_puppet_version which is a function that already exists in the spec_helper_acceptance.rb. This should resolve the failures and is only required in the acceptance tests.

Here is an example that I have done for size.rb: #925

Apologies!

@hlindberg
Copy link
Contributor Author

@pmcmaw PR updated, I changed all of the acceptance tests for functions checking with Puppet.version and 6.0.0

@hlindberg
Copy link
Contributor Author

@pmcmaw @clairecadman All requested changes are now in this PR. LGTM

@puppetlabs puppetlabs deleted a comment from hlindberg Jun 21, 2018
@pmcmaw
Copy link
Contributor

pmcmaw commented Jun 22, 2018

@clairecadman if you approve these changes I will get this merged 👍

@hlindberg
Copy link
Contributor Author

There are still the lower case 'puppet' in function files to address (I did not get that done yesterday, only fixed README.md). Merge and fix later, or fix now?

@pmcmaw
Copy link
Contributor

pmcmaw commented Jun 22, 2018

Ill do it later, would be nice to get this in 👍
Squash and merge isnt enabled for this repo (apparently?) So going to try to enable it then will get it in.

@clairecadman
Copy link
Contributor

clairecadman commented Jun 22, 2018 via email

@hlindberg
Copy link
Contributor Author

Was easily done with a global search. Added a commit.

@hlindberg
Copy link
Contributor Author

@pmcmaw The many commits are done on purpose as it makes it easier to later get relevant information when looking at "blame" for some lines (they have link to the puppet tickets that caused the doc change for example). A squash will give you a commit message containing all of the comments I made about each change for those lines. Anyway, I don't object to squashing, just my 2c for why I did many commits.

@pmcmaw pmcmaw merged commit f12e494 into puppetlabs:master Jun 22, 2018
@pmcmaw pmcmaw added the bugfix label Jun 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants