(PDK-842) Wire puppet-version and pe-version options into subcommands

[scotje]

Unless otherwise stated, packaged install stuff requires packages built from the HEAD of master on pdk-vanagon.

[coveralls]

Coverage decreased (-0.02%) to 92.235% when pulling 1f56df6 on scotje:842_its_aliveeeee into 101ef66 on puppetlabs:master.

[rodjek]

@scotje I've made some changes to get the existing unit and acceptance tests to pass with these changes. The only major behavioural change that you might want to review is in 284c693

[scotje]

Hmm, that's odd. If all_deps_available was true then bundle lock --update shouldn't reach out to rubygems for updates since we are passing the --local flag. This shouldn't hurt anything though but it is curious.

[rodjek]

So, this is what it'd look like with --puppet-version/--pe-version as global flags for use in pdk bundle. This command being special and skipping argument parsing would require the option to come between pdk and bundle (as everything after bundle is considered an argument).

semirhage :0: pdk/foo (git:842_its_aliveeeee → scotje U:4 ?:1!)$ ../bin/pdk --puppet-version 4.7.0 bundle exec puppet --version
pdk (INFO): Using Ruby 2.1.10
pdk (INFO): Using Puppet 4.7.0
[✔] Installing missing Gemfile dependencies.
4.7.0

semirhage :0: pdk/foo (git:842_its_aliveeeee → scotje U:4 ?:1!)$ ../bin/pdk --puppet-version 5.5.0 bundle exec puppet --version
pdk (INFO): Using Ruby 2.1.10
pdk (INFO): Using Puppet 5.5.0
5.5.0

I'm honestly not sure if it's worth it to have the specific option ordering for this command vs using an env var to set the desired puppet version for pdk bundle... I guess consistency would be the main point in its favour. I'll leave it for you @scotje and @bmjen to discuss and squash or drop the commit as appropriate :)

[scotje]

My vote would be to recognize values from PDK_PUPPET_VERSION and PDK_PE_VERSION environment variables and not make the --puppet-version and --pe-version options global.

My reasoning:

• Long term I think we should be aiming to remove the bundle subcommand. It's an escape hatch for use cases we haven't yet considered or addressed but part of the whole reason for PDK existing is to make it unnecessary for people to even be aware of Bundler.
• Making option strings more positional is bad for UX.
• Having those options show up in the help output for subcommands where they have no impact is also bad for UX.

[bmjen]

@rodjek IMO, we shouldn't bother with adding those flags to pdk bundle or anywhere other than validate and test unit. With pdk bundle being an experimental command, it doesn't seem necessary. It may also cause complications when users traditionally are used to passing in session vars to bundler like PUPPET_GEM_VERSION=file:///puppet pdk bundle update to do some more advanced use cases.

[bmjen]

This commented needs to be removed or a reason for leaving it should be documented.

[scotje]

Oh yeah I've been meaning to remove this line.

[bmjen]

Aside from my commit.. this LGTM with the latest changes.