Skip to content

(#22789) Add vserver_host in physical_types#547

Closed
tampakrap wants to merge 2 commits intopuppetlabs:masterfrom
tampakrap:22789_vserver_host
Closed

(#22789) Add vserver_host in physical_types#547
tampakrap wants to merge 2 commits intopuppetlabs:masterfrom
tampakrap:22789_vserver_host

Conversation

@tampakrap
Copy link

virtual => vserver_host was incorrectly shown as is_virtual => true
Adding it to physical_types variable so as it is properly shown as is_virtual => false

virtual => vserver_host was incorrectly shown as is_virtual => true
Adding it to physical_types variable so as it is properly shown as is_virtual => false
@puppetcla
Copy link

CLA signed by all contributors.

@adrienthebo
Copy link

Thank you very much for this contribution!

Since this is a backwards compatible bugfix, it should probably be targeted against stable. Right now Github doesn't permit changing the target branch for a pull request so we'll need to manually merge this against stable.

Since this is a behavior change that fixes a bug, we would like this pull request to include tests. This helps ensure that the change does what we expect and will help prevent regressions in the future. We have existing tests like https://github.com/puppetlabs/facter/blob/stable/spec/unit/virtual_spec.rb#L435 that could be copied and modified for this change, is this something you would be comfortable doing?

@tampakrap
Copy link
Author

Added tests, and confirmed that the second one returns Failure in the previous state

They ensure that proper `virtual` and `is_virtual` values are returned
@adrienthebo
Copy link

summary: merged into stable in 303d3ed and merged up into master in 303d3ed; this should be released into 1.7.4 and 2.0.0. Thanks again for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants