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

Pipe was broken in the previous rubucop fix #243

Merged
merged 1 commit into from
Sep 25, 2016
Merged

Pipe was broken in the previous rubucop fix #243

merged 1 commit into from
Sep 25, 2016

Conversation

asasfu
Copy link
Contributor

@asasfu asasfu commented Sep 25, 2016

Pipe was being set previously, 2 commits back, but then not used so rubucop complained. 1 commit back, pipe was then changed to being compared, which broke it because it was never set. This fixes that to remove the variable pipe altogether since we only use @resource[:pipe] once and don't need a variable for it. Brings it in line with things like @resource[:source]

Fixes #242

Pipe was being set previously, 2 commits back, but then not used so rubucop complained.  1 commit back, pipe was then changed to being compared, which broke it because it was never set.  This fixes that to remove the variable `pipe` altogether since we only use @resource[:pipe] once and don't need a variable for it.  Brings it in line with things like @resource[:source]
@alexjfisher alexjfisher merged commit 03411b6 into voxpupuli:master Sep 25, 2016
@asasfu asasfu deleted the patch_issue242_brokenprovider branch September 25, 2016 13:55
@alexjfisher
Copy link
Member

@asasfu Thanks for spotting my slip up. I suspected I'd probably messed up somewhere during the rubocop fixes, but couldn't seem to reproduce #242 when I tried this morning.

I hope to be able to add some acceptance tests soon.

@asasfu
Copy link
Contributor Author

asasfu commented Sep 25, 2016

You're welcome, I just only realized that this module has finally been resurrected in commits and you guys are doing a bunch of great work getting a lot more patched in. I've still yet to learn how to write tests, so i'll bow out of that task ;)

@alexjfisher
Copy link
Member

@asasfu The last rubocop fixes are in #241 Would you mind taking a look?

There are 2 types of test that could be added. We could add unit tests for the providers, or acceptance tests using docker. I'm looking forward to see what happens with voxpupuli/modulesync_config#230

@asasfu
Copy link
Contributor Author

asasfu commented Sep 25, 2016

@alexjfisher I Think #241 looks good. I was fairly thorough on my checking, but i'm not a professional programmer either :)

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.

Missing local variable 'pipe' on PECL extension installation
2 participants