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

(maint) Fix issue #98, don't require pe_compiling_server_aio_build #100

Merged
merged 1 commit into from
Apr 6, 2016

Conversation

MikaelSmith
Copy link
Contributor

Compilation should not require pe_compiling_server_aio_build. Skip
calling it if not defined even if is_pe is true.

true => pe_compiling_server_aio_build(),
default => undef,
if ($_is_pe && is_function_available('pe_compiling_server_aio_build')) {
$master_agent_version = pe_compiling_server_aio_build()
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add an else to explicitly set it to undef? We're going to get warnings all over the place otherwise.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. I also want to add a spec test.

@MikaelSmith
Copy link
Contributor Author

Updated.

@highb
Copy link
Contributor

highb commented Apr 5, 2016

👍 once specs pass.

@MikaelSmith
Copy link
Contributor Author

Specs are proving tricky.

…io_build`

Compilation should not require `pe_compiling_server_aio_build`. Skip
calling it if not defined even if `is_pe` is true.
@MikaelSmith
Copy link
Contributor Author

I think I got all the tests working. Got a little complicated.

@highb
Copy link
Contributor

highb commented Apr 6, 2016

Heh, that did get a bit complicated. Still, the fundamental change make sense and the specs mostly cover its behavior so 👍

@highb highb merged commit 410a7a6 into puppetlabs:master Apr 6, 2016
@MikaelSmith MikaelSmith deleted the issue-98 branch April 6, 2016 20:16
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