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

allow pipe param for pecl extensions #297

Merged
merged 1 commit into from Jan 19, 2017
Merged

allow pipe param for pecl extensions #297

merged 1 commit into from Jan 19, 2017

Conversation

@@ -66,6 +70,7 @@
$settings = {},
$settings_prefix = false,
$sapi = 'ALL',
$pipe = undef,
Copy link
Member

Choose a reason for hiding this comment

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

Does $pipe then not also have to be passed as an argument to ensure_packages?

Copy link
Author

Choose a reason for hiding this comment

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

Thanks! I ran the wrong test on my side and missed this. PR updated

@@ -52,6 +52,10 @@
# String parameter, whether to specify ALL sapi or a specific sapi.
# Defaults to ALL.
#
# [*pipe*]
# String parameter that may be used by the *provider* to input answers during
Copy link
Member

Choose a reason for hiding this comment

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

Maybe mention it's for the 'pecl' provider only here?

Copy link
Author

Choose a reason for hiding this comment

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

Hoping this will be updated correctly in the future, sure :)

Copy link
Sponsor Member

@rnelson0 rnelson0 left a comment

Choose a reason for hiding this comment

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

The current rubocop error is due to a modulesync issue and should not hold up the merge, once @alexjfisher's concerns have been addressed.

@alexjfisher alexjfisher merged commit 79ae3cd into voxpupuli:master Jan 19, 2017
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.

None yet

2 participants