(CLOUD-1978) pdk changes for k8 module #148
Conversation
| env: | ||
| - PUPPET_GEM_VERSION="~> 4.0" |
There was a problem hiding this comment.
You previously ran unit tests against both Puppet 4 and 5 in travis, maybe something you want to add back in via sync.yml?
There was a problem hiding this comment.
@HelenCampbell Thanks for the valuable comment.It added both env puppet version 4 and 5 since default rvm version is added is 2.1.9 modified the travis .yaml file to use 2.3.3 and travis is running on both version now.
.sync.yml
Outdated
| Gemfile: | ||
| required: | ||
| ':system_tests': | ||
| - gem: 'nokogiri' |
There was a problem hiding this comment.
You may want to re-visit the location of some of these Gems, e.g. nokogiri would be probably more suited towards 'development'. Although it's not breaking tests now we don't use 'without system tests', this might be introduced back in the future and you could hit the same issue.
There was a problem hiding this comment.
@HelenCampbell moved gems to development.Modified .sync.yaml file
spec/spec_helper.rb
Outdated
| require 'puppetlabs_spec_helper/module_spec_helper' | ||
| require 'simplecov' |
There was a problem hiding this comment.
The change in this file will remove your code coverage, just thought I'd call this out incase it was unintended.
There was a problem hiding this comment.
@HelenCampbell Thanks for the valuable comments. Incorporated the review comment.
|
Looks awesome @sheenaajay :D LGTM! |
| Gemfile: | ||
| required: | ||
| ':development': | ||
| - gem: 'nokogiri' |
There was a problem hiding this comment.
Some of these gems aren't necessary because they will be defined in the puppet-module-gems, which are the meta gems that were created to manage gem dependencies without having to release a new version of a module each time a pin is added.
Take a look at the list of defined dependencies here: https://github.com/puppetlabs/puppet-module-gems/blob/master/config/dependencies.yml
There is a matrix defined for windows, posix, and shared for 4 different versions of Ruby. Things like metadata-json-lint, puppet-lint, puppetlabs_spec_helper.. don't need to be defined explicitly in your Gemfile anymore.
There was a problem hiding this comment.
@bmjen Thanks Bryan. Will take a look and incorporate the comments.
There was a problem hiding this comment.
@bmjen Thanks alot for the valuable comments. Incorporated all comments.
.sync.yml
Outdated
| - gem: 'nokogiri' | ||
| version: '1.8.4' | ||
| - gem: 'net-telnet' | ||
| version: '0.2.0' |
There was a problem hiding this comment.
Does this module support Ruby 2.1.9? This version of net-telnet removes support for 2.1.9. I also believe this will conflict with the pin that the puppet-module-gems have of net-telnet at 0.1.1. So if you don't explicitly need a feature from 0.2.0, it's probably best to remove this and use the meta-gems pin.
There was a problem hiding this comment.
@bmjen this module doesn't support 2.1.9 version.Sure taking a look now.
Request to review the changes for pdk convert of Kubernetes module
Manual changes are required for the following files.
.rubocop.yml (modify the TargetRubyVersion to 2.2 and adding more Exclude directories)
Gemfile (to add a conditional clause for puppet_gem_version and ruby)
.travis to remove the values from the matrix (rvm: 2.1.9) since for K8 we require 2.3. and above
the commands to run the module will be
pdk validate ruby lib --puppet-version='5.3.6'
pdk validate puppet --puppet-version='5.3.6'
pdk validate metadata --puppet-version='5.3.6'
pdk test unit --puppet-version '5.3.6'
won't be able to run puppet version 4.10.11 since it uses ruby version 2.1.9 and we need 2.3 and above for this module