Skip to content
This repository has been archived by the owner on Jun 5, 2020. It is now read-only.

Addition of Ruby 2.2 and 2.3 #69

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

simmerz
Copy link

@simmerz simmerz commented Oct 5, 2016

This PR adds Rubies 2.2 and 2.3.

.travis.yml Outdated
@@ -10,5 +10,9 @@ matrix:
env: PUPPET_GEM_VERSION="~> 3.0"
- rvm: 2.1.8
env: PUPPET_GEM_VERSION="~> 3.0"
- rvm: 2.2.5
env: PUPPET_GEM_VERSION="~> 4.0"
- rvm: 2.3.0
Copy link
Contributor

Choose a reason for hiding this comment

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

It was announced that the new AIO would use ruby 2.3.1, would use that version here.

@@ -140,6 +140,18 @@
warning('Packages for Ruby 2.1 are not available from default repositories.')
}
}
/^2\.2.*$/:{
Copy link
Contributor

Choose a reason for hiding this comment

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

if the regex is meant to match 2.2.* then it should be /^2\.2\..*$/

Copy link
Author

Choose a reason for hiding this comment

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

This copies the same structure as 2.1 above it. Should they all be changed?

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like. Cause the current regex would also match 2.20.0, which is the not what you want.

Copy link
Contributor

Choose a reason for hiding this comment

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

That means there should also be checks to ensure that 2.2.1 does a thing and 2.20.1 does not do that thing.

Copy link
Author

Choose a reason for hiding this comment

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

Added a better regex across the board.

warning('Packages for Ruby 2.2 are not available from default repositories.')
}
}
/^2\.3.*$/:{
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above

tests/ruby2.3.pp Outdated
@@ -0,0 +1,4 @@
class{'ruby':
Copy link
Contributor

Choose a reason for hiding this comment

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

class { 'ruby':`

@@ -654,6 +654,68 @@
}
end

describe 'with ruby 2.2 with switch' do
Copy link
Contributor

Choose a reason for hiding this comment

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

spec tests are missing Ubuntu testing which is referenced in the manifest.

Copy link
Author

Choose a reason for hiding this comment

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

It seems Travis fails quite catastrophically on Puppet 4.x and Rubies 2.2 and 2.3. Not sure where to go from here.

Choose a reason for hiding this comment

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

Technically you won't need to spec test against ruby 2.2 or 2.3 on Puppet 4 yet because it still uses 2.1 internally I believe.

@simmerz
Copy link
Author

simmerz commented Oct 5, 2016

I'm unsure how to proceed here to get the Puppet 4.x / Ruby 2.3 tests passing at all. I presumed I needed to add to the travis file because otherwise we can't test under that ruby, but maybe that doesn't matter.

@rendhalver
Copy link

Just because you are installing ruby 2.2 and 2.3 it doesn't mean you need your module to pass tests for ruby 2.2 and 2.3.
Puppet 4 doesn't use system ruby and it's internal ruby is still on 2.1 last I looked.

@simmerz
Copy link
Author

simmerz commented Dec 22, 2016

True, so I can remove those travis tests and all will be well?

@rendhalver
Copy link

Yep. In theory anyway. :)

@simmerz
Copy link
Author

simmerz commented Dec 22, 2016

There we go. Passing test suite

Copy link

@selyx selyx left a comment

Choose a reason for hiding this comment

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

remove unused group matches from regular expressions

@@ -119,27 +119,39 @@
}
default:{
case $version {
/^1\.8.*$/:{
/^1\.8(\..*)*$/:{
Copy link

Choose a reason for hiding this comment

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

since we don't use those match groups somewhere else we could simply leave them out

suggested: /^1\.8\..*$/

Copy link
Author

Choose a reason for hiding this comment

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

That implies that there will always be a '.' after the minor version. That's not always been the case afaik?

Copy link

Choose a reason for hiding this comment

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

As far as I know ruby devs are following semantic versioning which I think will always result in a version of the format $major.$minor.$patch.

But I just realized debian package names either contain two or three digits (e.g. ruby1.9.1, ruby2.1). Therefore your solution might be more logical for most users.

$real_ruby_package = "${ruby::params::ruby_package}1.8"
if ! $suppress_warnings and versioncmp($::operatingsystemrelease, '14.04') >= 0 {
warning('Packages for Ruby 1.8 are not available from default repositories.')
}
}
/^1\.9.*$/:{
/^1\.9(\..*)*$/:{
Copy link

Choose a reason for hiding this comment

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

/^1\.9\..*$/

Copy link
Author

Choose a reason for hiding this comment

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

Again, I'd be more inclined to change the other files to match what's currently in my PR - dev.pp is the only other place this happens.

@selyx
Copy link

selyx commented Feb 2, 2017

You should also add Ubuntu 16.04 to the supported operating systems in metadata.json

@simmerz
Copy link
Author

simmerz commented Feb 2, 2017

Not entirely sure why the tests are suddenly failing :\

@simmerz
Copy link
Author

simmerz commented Feb 3, 2017

1.9.3 removed because deps no longer build
4.9.0/Ruby 2.0.0 exception added because that also doesn't build, and 2.0.0 support is now deprecated.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants