Skip to content

Add a function to compare the OS version #972

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

Merged
merged 4 commits into from
Nov 27, 2018

Conversation

ekohl
Copy link
Collaborator

@ekohl ekohl commented Nov 23, 2018

This function aims to reduce the boiler plate that a lot of modules have
to compare versions:

if $facts['operatingsystem'] == 'Ubuntu' && versioncmp(facts['operatingsystemmajrelease'], '16.04') >= 0 {

Can now be reduced to:

if os_version_gte('Ubuntu', '16.04') {

Let the bikeshedding commence!

@david22swan
Copy link
Member

@ekohl Your change is failing some syntax checks, just a quick fix needed:

Offenses:
spec/functions/os_version_gte_spec.rb:7:9: C: Style/HashSyntax: Use hash rockets syntax. (https://github.com/bbatsov/ruby-style-guide#hash-literals)
        operatingsystem: 'Debian',
        ^^^^^^^^^^^^^^^^
spec/functions/os_version_gte_spec.rb:8:9: C: Style/HashSyntax: Use hash rockets syntax. (https://github.com/bbatsov/ruby-style-guide#hash-literals)
        operatingsystemmajrelease: '9',
        ^^^^^^^^^^^^^^^^^^^^^^^^^^
spec/functions/os_version_gte_spec.rb:22:9: C: Style/HashSyntax: Use hash rockets syntax. (https://github.com/bbatsov/ruby-style-guide#hash-literals)
        operatingsystem: 'Ubuntu',
        ^^^^^^^^^^^^^^^^
spec/functions/os_version_gte_spec.rb:23:9: C: Style/HashSyntax: Use hash rockets syntax. (https://github.com/bbatsov/ruby-style-guide#hash-literals)
        operatingsystemmajrelease: '16.04',

Otherwise, looks good :)

This function aims to reduce the boiler plate that a lot of modules have
to compare versions:

    if $facts['operatingsystem'] == 'Ubuntu' && versioncmp(facts['operatingsystemmajrelease'], '16.04') >= 0 {

Can now be reduced to:

    if os_version_gte('Ubuntu', '16.04') {
@david22swan
Copy link
Member

@ekohl Unfortunately some random failures have appeared in the spec/functions/to_yaml_spec.rb test file when run on appveyor. They are unrelated to your change however I am unable to merge while they are there, so I won't be able to merge this in until they are dealt with.

@ekohl
Copy link
Collaborator Author

ekohl commented Nov 23, 2018

I'm in no hurry :)

@ekohl
Copy link
Collaborator Author

ekohl commented Nov 23, 2018

Also realized this will also need docs in README.md I guess.

@david22swan
Copy link
Member

@ekohl The stdlib bug was fixed. If you could add some docs I will happily merge.

@david22swan
Copy link
Member

@ekohl Hope you don't mind but I added some docs changes myself. If you're ok with them thenI can get this merged.

@ekohl
Copy link
Collaborator Author

ekohl commented Nov 27, 2018

Fixed a typo, but otherwise looks good.

@eimlav eimlav merged commit d1706fe into puppetlabs:master Nov 27, 2018
@david22swan
Copy link
Member

@ekohl ++ :)

@david22swan
Copy link
Member

Thanks for the function

@ekohl ekohl deleted the os_version_gte branch November 27, 2018 13:51
@swegener
Copy link

The README.md says

Returns:
  - Boolean(0) # When OS is below the given version.
  - Boolean(1) # When OS is equal to or greater than the given version.

but the unit tests and the actual implementation behave like this:

        :operatingsystem => 'Debian',
        :operatingsystemmajrelease => '9',
[...]
    it { is_expected.to run.with_params('Debian', '8').and_return(false) }

Debian 9 is the OS version, but running os_version_gte with Debian 8 returns false.

@ekohl
Copy link
Collaborator Author

ekohl commented Sep 30, 2020

Hmm, you may be right. It would have been good to also test for Debian 10. I must admit I did not end up using this function myself. Could you provide a PR to better define the behavior and a bugfix if needed?

kenyon added a commit to kenyon/puppetlabs-stdlib that referenced this pull request Aug 27, 2021
As reported in
<puppetlabs#972 (comment)>,
os_version_gte is not returning correct results. This commit fixes the
tests to demonstrate correct expected behavior.
kenyon added a commit to kenyon/puppetlabs-stdlib that referenced this pull request Aug 27, 2021
As reported in
<puppetlabs#972 (comment)>,
os_version_gte is not returning correct results. This commit fixes the
tests to demonstrate correct expected behavior.
cegeka-jenkins pushed a commit to cegeka/puppet-stdlib that referenced this pull request Feb 21, 2024
As reported in
<puppetlabs/puppetlabs-stdlib#972 (comment)>,
os_version_gte is not returning correct results. This commit fixes the
tests to demonstrate correct expected behavior.
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.

5 participants