Skip to content
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

Ubuntu port #4

Merged
merged 7 commits into from
Dec 21, 2013
Merged

Ubuntu port #4

merged 7 commits into from
Dec 21, 2013

Conversation

kitchen
Copy link
Contributor

@kitchen kitchen commented Nov 19, 2013

this PR ports the module to ubuntu.

@jhoblitt
Copy link
Owner

Debian/Ubuntu support has been on my mental todo list forever, thank you for working on this!

On first glance, I'm OK with everything in this PR but I'd like to add a few basic rspec-system tests before merging it; I'll take an action item to do that.

Could you fix up the rspec-puppet tests? The tests in this module really need some TLC.

@jhoblitt
Copy link
Owner

Just to clarify, I'm only asking for the test failure to be resolved -- a test overhaul ;)

@kitchen
Copy link
Contributor Author

kitchen commented Nov 20, 2013

oh, hrm. I didn't even see the tests (I could have sworn I even looked!)

I'll fix that up for sure.

Also, I just realized that the gmetad init script also does not have status support so I'll add that to the PR as well in a bit.

Thanks!

@kitchen
Copy link
Contributor Author

kitchen commented Nov 20, 2013

and it looks like the status command bit has been fixed going forward (at least in 13.04) so I'll check and support that as well.

@kitchen
Copy link
Contributor Author

kitchen commented Nov 20, 2013

ok, should be ready to go again!

@kitchen
Copy link
Contributor Author

kitchen commented Nov 20, 2013

I checked the init script on 12.04, 12.10, and 13.04. 13.04 is when the status command comes in.

@jhoblitt
Copy link
Owner

Still has a travis failure.

$gmetad_service_erb = 'ganglia/gmetad.conf.el6.erb'

# ubuntu 12.10 and below didn't have a status command in the init script
if ($::lsbmajdistrelease <= 12) {
Copy link
Owner

Choose a reason for hiding this comment

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

This expression is causing a test failure with ruby 1.8.7 because the fact isn't declared; should be an easy fix. It would also be better to use $::operatingsystemmajrelease as that's a core fact not dependent on having an lsb package installed. If lsb* facts have to be used a module level dep on http://forge.puppetlabs.com/razorsedge/lsb should be introduced.

@kitchen
Copy link
Contributor Author

kitchen commented Nov 24, 2013

That's odd. It is declared on my ubuntu systems, whereas $::operatingsystemmajrelease is not.

@kitchen
Copy link
Contributor Author

kitchen commented Nov 29, 2013

yay, build passes now!

@jhoblitt
Copy link
Owner

jhoblitt commented Dec 9, 2013

Looks like I dropped the ball on this -- it's looks good now! I'm planning to merge it as soon as I find a few minutes to add rspec-system tests to the system with a ubuntu box.

@jhoblitt jhoblitt merged commit aed3ee5 into jhoblitt:master Dec 21, 2013
@jhoblitt
Copy link
Owner

I apologize again for taking so long on this. I finally found the time to properly test this and extend the trivial rspec-puppet test coverage to ubuntu. I plan to cut a 1.1.0 release shortly.

@kitchen
Copy link
Contributor Author

kitchen commented Dec 21, 2013

Rawr! Thanks :) sorry I've been quiet on this I actually switched to new relic so haven't had a direct interest to do some active development :) happy holidays!

-Jeremy

Insert PGP signature here.

On Dec 20, 2013, at 4:46 PM, Joshua Hoblitt notifications@github.com wrote:

I apologize again for taking so long on this. I finally found the time to properly test this and extend the trivial rspec-puppet test coverage to ubuntu. I plan to cut a 1.1.0 release shortly.


Reply to this email directly or view it on GitHub.

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.

2 participants