Modernise fqdn_rotate function#1341
Conversation
alexjfisher
requested review from
a team,
b4ldr,
bastelfreak,
ekohl and
smortex
as code owners
fqdn_rotate is a functionthat may have no external impact to Forge modules. stdlib::fqdn_rotate is a functionthat may have no external impact to Forge modules. fqdn_rotate is a functionthat may have no external impact to Forge modules. This module is declared in 318 of 580 indexed public
|
| # workaround not being able to use let(:facts) because some tests need | ||
| # multiple different hostnames in one context | ||
| allow(scope).to receive(:lookupvar).with('::fqdn').and_return(host) | ||
| allow(subject.func.closure_scope).to receive(:[]).with('facts').and_return({ 'networking' => { 'fqdn' => host } }) |
There was a problem hiding this comment.
What I really wanted to do was mock the function's private method fqdn_fact, but can't figure out how to do this.
alexjfisher
force-pushed
the
modernise_fqdn_rotate
branch
from
945bb0f to
f2f6635
Compare
|
Hey @alexjfisher, looks like there are some conflicts in the branch that need to be addressed before this can be merged. If you can get this sorted, the PR should be mergeable right away. |
|
Any progress with this PR, with the latest version we're currently still seeing lots of changes on each Puppet run as mentioned in #1381 |
| @@ -4,9 +4,9 @@ | |||
|
|
|||
| describe 'fqdn_rotate' do | |||
There was a problem hiding this comment.
| describe 'fqdn_rotate' do | |
| describe 'stdlib::fqdn_rotate' do |
There was a problem hiding this comment.
This was a deliberate choice to show that the shim does actually correctly invoke the namespaced function.
|
|
||
| function_args = [value] + extra | ||
| scope.function_fqdn_rotate(function_args) | ||
| scope.call_function('fqdn_rotate', function_args) |
There was a problem hiding this comment.
| scope.call_function('fqdn_rotate', function_args) | |
| scope.call_function('stdlib::fqdn_rotate', function_args) |
| repeated_param 'Any', :args | ||
| end | ||
| def deprecation_gen(*args) | ||
| call_function('deprecation', 'fqdn_rotate', 'This method is deprecated, please use stdlib::fqdn_rotate instead.') |
There was a problem hiding this comment.
With your work on #1377, I think it makes sense to:
| call_function('deprecation', 'fqdn_rotate', 'This method is deprecated, please use stdlib::fqdn_rotate instead.') | |
| call_function('deprecation', 'fqdn_rotate', 'This method is deprecated, please use stdlib::fqdn_rotate instead.', false) |
We may also open another PR to be merged when the next major version is released and that restore the failure?
There was a problem hiding this comment.
Fixed whilst rebasing by rerunning script.
alexjfisher
force-pushed
the
modernise_fqdn_rotate
branch
from
f2f6635 to
8190beb
Compare
|
Rebased to fix #1381 |
Convert the function to the modern function API as a namespaced function and use the `networking` fact instead of legacy facts. A non-namespaced shim is also created (but marked deprecated), to preserve compatibility. Fixes puppetlabs#1381
alexjfisher
force-pushed
the
modernise_fqdn_rotate
branch
from
8190beb to
bf32b58
Compare
ghost
mentioned this pull request
7 participants
Convert the function to the modern function API as a namespaced function and use the
networkingfact instead of legacy facts.A non-namespaced shim is also created (but marked deprecated), to preserve compatibility.
Along with #1326 this is the other half of the fix for #1315