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

Fixes #19016 - change qpid to localhost #115

Merged
merged 3 commits into from
Mar 29, 2017
Merged

Fixes #19016 - change qpid to localhost #115

merged 3 commits into from
Mar 29, 2017

Conversation

Klaas-
Copy link
Contributor

@Klaas- Klaas- commented Mar 24, 2017

No description provided.

@Klaas-
Copy link
Contributor Author

Klaas- commented Mar 24, 2017

Tests are broken for Puppet 3.5 (not by my change as far as I can tell)

@Klaas-
Copy link
Contributor Author

Klaas- commented Mar 24, 2017

I think xinetd module stopped supporting puppet3 :)

@ekohl
Copy link
Member

ekohl commented Mar 24, 2017

@Klaas- that's very possible, I know voxpupli and puppetlabs are working on it. It's why I've been pushing on dropping Puppet 3 after the Foreman 1.15 branching (which is soon).

@Klaas-
Copy link
Contributor Author

Klaas- commented Mar 27, 2017

@ekohl how to we continue here? it's failing because it pulls https://github.com/puppetlabs/puppetlabs-xinetd master and that one dropped puppet3 support. Should I specify a tag for that repo that works or is it acceptable to fail on the old puppet version?

@ehelms
Copy link
Member

ehelms commented Mar 27, 2017

Hrmm, we might be able to remove xinetd. This puppet module used to wrap foreman_proxy and thus required a lot of the fixtures it had. I'd bet a lot of those are not needed anymore.

@ekohl
Copy link
Member

ekohl commented Mar 27, 2017

The default puppet-foreman_proxy sets up tftp as well. We can set parameters to turn that off as well with the benefit of faster tests.

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

The code looks correct, but I'd like an ACK from @ehelms or @stbenjam about the actual change even though it makes sense.

@Klaas-
Copy link
Contributor Author

Klaas- commented Mar 29, 2017

@ekohl I fixed the new lint tests and the fixture for puppet3 :)
although I'm not sure why it enforces the ordering arrows to be on the other lines... :)

@ekohl
Copy link
Member

ekohl commented Mar 29, 2017

It's the new style since puppet-lint 2.2.0. We just merged a bunch of the changes for theforeman and we should do the same for katello.

@stbenjam
Copy link
Member

Ugh, I'd rather disable that lint check, it's hideous.

Copy link
Member

@stbenjam stbenjam left a comment

Choose a reason for hiding this comment

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

Change looks good to me

@ehelms
Copy link
Member

ehelms commented Mar 29, 2017

I agree, its like the leading comma's in JavaScript which were just silly IMO.

@ehelms ehelms merged commit c53398b into theforeman:master Mar 29, 2017
@Klaas- Klaas- deleted the Klaas-19016 branch April 3, 2017 09:01
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.

4 participants