(MODULES-9428) Clarify composite namevar edgecases and add examples #140
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hlindberg
suggested changes
May 24, 2019
clairecadman
approved these changes
May 24, 2019
hlindberg
reviewed
May 28, 2019
|
I'll come back to this after contributor summit. |
DavidS
force-pushed
the
clarify-composite-namevars
branch
from
7cfceec to
54d0eea
Compare
|
I've found a few more tickets that fold into the composite namevars issue that I'd like to bundle into this fix:
PDK-1220 is already covered in this text, the others need a bit more consideration. against my usual preferences in this repo, I've rebased and munged the commits on this change as far as it went to separate the editorial and the content changes. |
DavidS
changed the title
DavidS
force-pushed
the
clarify-composite-namevars
branch
from
54d0eea to
1ea497b
Compare
Break up the dense information from the paragraph into a bulleted list. This ensures that the different statements do not run into each other.
Improve the initial type schema example.
…e less duplication
* Point out that there can be more than two namevars * Improve composite namevar example This is rewording doc strings, fixing syntax errors, and reformatting the example. * Clarify behaviour when no `title_pattern` is specified Make sure to communicate the behaviour and consequences when no `title_patterns` are specified. * Upgraded the "order" note to a definitive statement on the matching behaviour. * Update the description for `get` and `set` for composite namevars This moves the existing explanation for how `get` should behave to the definition of the method and adds examples for the values passed around by `get` and `set`. * Mention again that composite namevars are supported by SimpleProvider * Remove the `title` key from the hash being passed to SimpleProvider methods Since we can't guarantee to always have a title available, we can't promise to pass one through to the implementation. * Add a note and examples about `names` usage in `simple_get_filter` This also covers PDK-1219, PDK-1220, and PDK-1221.
DavidS
force-pushed
the
clarify-composite-namevars
branch
from
1ea497b to
8df5350
Compare
|
@Lavinia-Dan @da-ar this is now in a shape to review. Note that the commits are broken up into distinct chunks that should help reduce clutter during reviewing. |
da-ar
reviewed
Jun 19, 2019
The remote_resource example had remnants from a supports_noop implementation. This change removes those remnants as they are misleading and do not add value to this example.
hlindberg
suggested changes
Jun 19, 2019
* Fixed quoting in examples * Improve wording in a couple of places
After conversation with Henrik, I'm certain that we should not burden the text or code with the complexity of having title_patterns optional.
|
Ok, I think this wraps it up. The implementation is available at puppetlabs/puppet-resource_api#174 and there don't seem to be further comments here. |
da-ar
approved these changes
Jun 28, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.