(GH-225) Add custom_insync to rsapi docs #153
Conversation
language/resource-api/README.md
Outdated
| end | ||
| ``` | ||
|
|
||
| In normal operations, the runtime environment uses strict value equality to determine if a resource instance needs to be state-enforced. |
There was a problem hiding this comment.
Does this mean without using this new feature?
There was a problem hiding this comment.
Maybe say "When you don't use "custom_insync, the runtime environment etc...."?
There was a problem hiding this comment.
I believe I've addressed this by breaking the docs up a little to clarify:
| As with all calls to `insync?`, a custom change reporting message can be surfaced by returning a string instead of `true` or `false`. | ||
| If no custom change reporting message is returned as a string, the change reporting in Puppet (if `false` is returned) will always be: | ||
|
|
||
| > Custom insync logic determined that this resource is out of sync |
There was a problem hiding this comment.
Is this the message the user would see?
| ) | ||
|
|
||
| # lib/puppet/provider/test_custom_insync_hidden_property/test_custom_insync_hidden_property.rb | ||
| class Puppet::Provider::TestCustomInsyncHiddenProperty::TestCustomInsyncHiddenProperty |
There was a problem hiding this comment.
What is this example showing compared to the first one?
There was a problem hiding this comment.
This shows an example without an insyncable property - which adds a hidden property to ensure that insync? is triggered.
There was a problem hiding this comment.
Ah okay, maybe make that clearer?
There was a problem hiding this comment.
I've moved this entire section into a subheading, does that address this concern?
There was a problem hiding this comment.
Yes that is better, thanks!
|
@clairecadman just pinging to see if my updates addressed your review comments :) |
|
@michaeltlombardi yes all good thanks! I don't want to change anything else as it would make it inconsistent with the rest of the README. |
This commit coincides with the work for puppetlabs/puppet-resource_api#225 to add the new custom_insync feature and functionality to rsapi resources; It covers a common use case in the code and explains the purpose and some implementation concerns.
| If no properties were specified in the instance of a resource, `insync?` is not called. | ||
| The call happens during application of the catalog when the runtime environment decides whether or not it needs to enforce state on a specific resource. | ||
|
|
||
| The `insync?` method **must** return `nil`, `true` or `false` as the result of the property comparison: |
There was a problem hiding this comment.
this is not explaining the new API requested by @joshcooper
There was a problem hiding this comment.
I think this is resolved by the immediately following block which explains that the message only matters if the result is actually false:
|
@clairecadman, @DavidS, can we get a final approval and merge on this if there's nothing else? |
This PR coincides with the work for puppetlabs/puppet-resource_api#225
to add the new custom_insync feature and functionality to rsapi resources;
It covers a common use case in the code and explains the purpose and some
implementation concerns.