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

Refactor plugin parameter handling #500

Merged
merged 3 commits into from
Dec 26, 2019
Merged

Conversation

ekohl
Copy link
Member

@ekohl ekohl commented Apr 3, 2019

When all values are static, there's no need to load it via a params class. It also tightens some parameter types.

@ekohl ekohl changed the title Ensure the Infoblox DNS server is a valid host Refactor plugin parameter handling Apr 3, 2019
@mmoll
Copy link
Contributor

mmoll commented Apr 5, 2019

@ekohl tests fail

@ekohl ekohl force-pushed the infoblox-dns branch 3 times, most recently from 9db590a to d40c691 Compare July 2, 2019 11:05
@mmoll
Copy link
Contributor

mmoll commented Dec 22, 2019

@ekohl needs a rebase ;)

@ekohl
Copy link
Member Author

ekohl commented Dec 26, 2019

Rebased. Also includes some spec improvements as separate commits to speed up the testing.

@mmoll
Copy link
Contributor

mmoll commented Dec 26, 2019

Fails on Puppet 5 💔

Passing in a supported_os array instead of filtering the result is much
faster because it doesn't have to load the entire database.
This mechanism is custom and not used. rspec-puppet-facts has
SPEC_FACTS_OS to filter which has the added benefit that it filters
natively and is faster.

It's not removed from spec_helper.rb because that's rsynced.
When all values are static, there's no need to load it via a params
class. It also tightens some parameter types.
@ekohl
Copy link
Member Author

ekohl commented Dec 26, 2019

Looks like this reduces the unit test time from 21 to 26 minutes (https://travis-ci.org/theforeman/puppet-foreman_proxy/builds/629474276) to 6 to 7 minutes (https://travis-ci.org/theforeman/puppet-foreman_proxy/builds/629687391).

@mmoll mmoll merged commit 402d6f6 into theforeman:master Dec 26, 2019
@mmoll
Copy link
Contributor

mmoll commented Dec 26, 2019

merged, bedankt @ekohl!

@ekohl ekohl deleted the infoblox-dns branch December 26, 2019 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants