-
-
Notifications
You must be signed in to change notification settings - Fork 269
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
use Stdlib::Fqdn type for package_keyserver / bump stdlib to 4.25 #759
use Stdlib::Fqdn type for package_keyserver / bump stdlib to 4.25 #759
Conversation
I restarted the travis jobs. @marcdeop https://forge.puppet.com/puppetlabs/stdlib got released 3 days ago. Can you please bump the required version in metadata.json? |
aff071d
to
c0c5ba9
Compare
Commit 7ae6079 bumps minimum stdlib version both in metadata.json and in .fixtures.yml so we can make sure that tests run the same way as the production code :-) |
.fixtures.yml
Outdated
@@ -4,6 +4,7 @@ fixtures: | |||
repo: 'git://github.com/puppetlabs/puppetlabs-apt' | |||
stdlib: | |||
repo: 'git://github.com/puppetlabs/puppetlabs-stdlib' | |||
ref: "4.25.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please don't bump it here. We always want to test against the master branch to catch unreleased errors early.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed it from the commit even though I don't understand the reason.
Which unreleased errors? From the stdlib library? That would not make much sense...
I can only see downsides on doing this: you are testing with code that it will be different from what the actual production code will be based on. How does that make sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's a complicated story. In the past our modules broke often because a breaking change was introduced into a dependency, back then we references releases in the .fixtures.yml. We pinned it to the lowest known working version. The puppet module tool always tries to download the latest version for dependencies.
tldr: We noticed that dependencies aren't compatible in the latest version only because it failed for our users and they reported it.
What we now do is a bit smarter. We always test against the master branches of our dependencies. So we notice during our regular PRs to our modules, if a dependency isn't working anymore. This is all related to our unit tests.
Our acceptance tests are different. We start docker instances on travis, install dependencies from our metadata.json, execute puppet code and check if everything is working. We're simulating our users.
Combination of unit and acceptance tests are, in my opinion, the best way to address/track breaking changes.
This was a rather long explanation and I'm not a native speaker. If this text raises more questions than it answers please feel free to join our IRC channel #voxpupuli on freenode and we can talk about this.
7ae6079
to
de0a96e
Compare
As discussed #746 here is a PR to fix the data type for package_keyserver.
This is cannot be merged until there is a release of puppetlabs-stdlib that include the proper datatype (it's only on master right now)