-
Notifications
You must be signed in to change notification settings - Fork 611
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
Add default values for more openSUSE and SLES distro versions #615
Conversation
d5c513f
to
15a6d75
Compare
I don't think the test failures are related to my changes |
15a6d75
to
44ffcc1
Compare
@@ -94,7 +94,13 @@ | |||
'FreeBSD' => '93', | |||
'OpenBSD' => '9.3', | |||
'Suse' => $::operatingsystem ? { | |||
'SLES' => '91', | |||
'SLES' => $::operatingsystemrelease ? { | |||
'11.3' => '91', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be the default
case, for backwards compatibility
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 after default added
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the default => '91'
would be wrong though eg on SLES11SP2 (and maybe on the upcoming SLES12SP1, I need to check it). How about default => 'undef'
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wrong, SLES11SP2 also has '91', so I changed the default
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line can be removed now? Also, how is this working in future versions of SLES? Will 12.1 have 91 again, or does that need another patch?
Using versioncmp would make that much nicer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took the liberty to use 93 as default, and assign the 11.x distros to 91 through a regexp. Now it covers all the 11.x series (along with the upcoming sle11sp4) and also sle12 and the upcoming sle12sp1
44ffcc1
to
49a0385
Compare
49a0385
to
5ddc889
Compare
Add default values for more openSUSE and SLES distro versions
Add default values for more openSUSE and SLES distro versions
No description provided.