Skip to content

Conversation

@bittner
Copy link
Contributor

@bittner bittner commented Dec 12, 2017

These changes add the free VMware Workstation Player to the virtualization class.

Fixes issue #12.

@bittner
Copy link
Contributor Author

bittner commented Dec 13, 2017

The build fails only for things that will go away with using PDK. Same as in #16 and #18.

@bittner
Copy link
Contributor Author

bittner commented Feb 2, 2018

Any chance of this getting merged?

@bittner
Copy link
Contributor Author

bittner commented Apr 4, 2018

🔔 ping!

@edestecd
Copy link
Owner

Can you rebase and make sure the tests pass now?

…into feature/add-vmware-player-to-virtualization
@bittner bittner force-pushed the feature/add-vmware-player-to-virtualization branch from ea45b43 to 1dae792 Compare May 23, 2018 21:21
@bittner bittner force-pushed the feature/add-vmware-player-to-virtualization branch from 7d17314 to e39396b Compare May 23, 2018 21:48
@bittner
Copy link
Contributor Author

bittner commented May 23, 2018

All lights are green, yay!

Sorry that I just pulled instead of rebasing. I hope it's fine anyway.

@bittner
Copy link
Contributor Author

bittner commented Jun 10, 2018

This is ready for merging. ping! 🔔


case $::operatingsystem {
'Debian', 'Ubuntu': {
validate_string($version, $urlbase, $package)
Copy link
Owner

Choose a reason for hiding this comment

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

Can we switch to puppet 4 type validations?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What would that look like? (Sorry!)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, type hinting, okay. That's the somewhat

class software::virtualization::vmware (
  Enum['present', 'absent'] $ensure = $software::params::software_ensure,
  String $version = $software::params::vmware_version,
  String $url = $software::params::vmware_urlbase,
  String $package = $software::params::vmware_package,
) inherits software::params {

There may be some types better suited for (semantic) version and URL than String, and Optional[] probably needs to be wrapped around some. What's your preference?

Hmmm ... (I'm trying to avoid the "I'm away from Puppet, currently" argument) 😏 🤐 😳

Copy link
Owner

Choose a reason for hiding this comment

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

Stdlib has some types, but other than that I usually stick with the built in ones.
https://github.com/puppetlabs/puppetlabs-stdlib/tree/master/types


$vmware_installer = "/tmp/${package}"
$vmware_install_cmd = "${vmware_installer} --console --eulas-agreed --required"
$vmware_uninstall_cmd = "${vmware_installer} --uninstall-product=vmware-player --required"
Copy link
Owner

Choose a reason for hiding this comment

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

IS this variable ever used? If you want it around for reference, use a comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should ideally run this when ensure is absent.

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.

2 participants