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

(FM-8104) Add implicit default values #92

Merged
merged 2 commits into from
Jun 5, 2019
Merged

Conversation

shermdog
Copy link
Contributor

This PR adds implicit default values to resource output for panos_nat_policy panos_security_policy_rule panos_static_route_base
This is not exhaustive and additonal providers may need to be updated.
This PR also corrects some data types in panos_static_route

@@ -71,15 +71,15 @@
xpath: 'interface/text()',
},
metric: {
type: 'Optional[String]',
desc: 'Specify a valid metric for the static route (1 - 65535).',
type: 'Integer[1, 65535]',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these type changes are breaking interface changes. Please change this to accept Variant[String, Integer[..]] and add a canonicalize method to the provider that transforms the value to the preferred form.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If someone from the eng team can pick this up and round it out that would be super. I'm running full speed on my demo work.

This PR adds implicit default values to resource output for panos_nat_policy panos_security_policy_rule panos_static_route_base
This is not exhaustive and additonal providers may need to be updated.
This PR also corrects some data types in panos_static_route
@codecov
Copy link

codecov bot commented May 24, 2019

Codecov Report

Merging #92 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #92      +/-   ##
==========================================
+ Coverage   99.27%   99.28%   +<.01%     
==========================================
  Files          40       40              
  Lines        1102     1114      +12     
==========================================
+ Hits         1094     1106      +12     
  Misses          8        8
Impacted Files Coverage Δ
lib/puppet/type/panos_static_route.rb 100% <ø> (ø) ⬆️
lib/puppet/type/panos_ipv6_static_route.rb 100% <ø> (ø) ⬆️
...ppet/provider/panos_nat_policy/panos_nat_policy.rb 100% <100%> (ø) ⬆️
...security_policy_rule/panos_security_policy_rule.rb 100% <100%> (ø) ⬆️
lib/puppet/provider/panos_static_route_base.rb 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d91228f...f0e5074. Read the comment docs.

@shermdog
Copy link
Contributor Author

shermdog commented Jun 4, 2019

@da-ar changes look good. I suppose at some point we'll want to add deprecation warnings and push users to using stricter types to get the benefits of validation. Thank you for closing this out!

@da-ar da-ar requested a review from DavidS June 5, 2019 08:58
@da-ar da-ar merged commit 50ba3ca into puppetlabs:master Jun 5, 2019
@da-ar da-ar added the feature New feature or request label Jun 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants