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

(PDK-785) Add --puppet-version and --pe-version CLI options #448

Merged
merged 1 commit into from
Mar 19, 2018

Conversation

rodjek
Copy link
Contributor

@rodjek rodjek commented Mar 8, 2018

The important bit: It adds the --puppet-version and --pe-version options to the pdk validate and pdk test unit commands (it's a lot of changed lines for such a simple change, but this PR lays a lot of the groundwork for the rest of the puppet & ruby version switching behaviour).

  • PE versions are mapped to Puppet versions in a static hash (copied from the forge), though this will eventually be made dynamic.
  • GEM_PATH and GEM_HOME have been extracted from PDK::CLI::Exec into a separate class, allowing them to be reused when scanning for available Puppet versions (and eventual ruby version switching).
  • Available Puppet versions are determined by scanning GEM_HOME and GEM_PATH for puppet gemspec files and parsing their contents.
  • Puppet version selection will prefer an exact match if available, otherwise it'll use the latest patch release for the specified version.

I would recommend not being too worried about the internals of PDK::Util::PuppetVersion and PDK::Util::RubyVersion at this stage, as they will change as this feature evolves in further PRs.

@coveralls
Copy link

coveralls commented Mar 8, 2018

Coverage Status

Coverage increased (+0.3%) to 92.421% when pulling 4b5c441 on rodjek:pdk-785 into f28a9ad on puppetlabs:master.

@rodjek rodjek force-pushed the pdk-785 branch 3 times, most recently from 0f95455 to c9b9255 Compare March 19, 2018 02:42
@rodjek rodjek changed the title [WIP] (PDK-785) Initial spike of Puppet version munging (PDK-785) Initial spike of Puppet version munging Mar 19, 2018
@rodjek rodjek requested review from scotje and bmjen March 19, 2018 03:05
@rodjek rodjek changed the title (PDK-785) Initial spike of Puppet version munging (PDK-785) Add --puppet-version and --pe-version CLI options Mar 19, 2018
Copy link
Contributor

@scotje scotje left a comment

Choose a reason for hiding this comment

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

Looks good Tim! Like you say, some of the internals will probably get revised as we go along but this seems like a great start.

@@ -21,6 +22,11 @@ module PDK::CLI
run do |opts, _args, _cmd|
require 'pdk/tests/unit'

if opts[:'puppet-version'] && opts[:'pe-version']
PDK.logger.error _('You can not specify both --puppet-version and --pe-version at the same time.')
exit 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this raise PDK::CLI::ExitWithError instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call

@@ -32,6 +33,11 @@ module PDK::CLI
exit 0
end

if opts[:'puppet-version'] && opts[:'pe-version']
PDK.logger.error _('You can not specify both --puppet-version and --pe-version at the same time.')
exit 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this raise PDK::CLI::ExitWithError instead?

@rodjek rodjek merged commit 127450f into puppetlabs:master Mar 19, 2018
@rodjek rodjek deleted the pdk-785 branch March 19, 2018 22:11
@bmjen bmjen added the feature label Apr 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants