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

Correctly detect Open3#capture3 existance. #34

Closed
wants to merge 2 commits into from

Conversation

fbernier
Copy link

The commit message is not exactly right, but the fact is that method_defined? will tell you whether INSTANCES of the class with the module included responds to the given method, which is not the case for Open3#capture3. We have to use respond_to? for this case. Before this commit, your code was always using your bundled version of capture3.

@fbernier fbernier closed this Jan 11, 2013
@davetapley
Copy link
Contributor

@fbernier @csquared Why wasn't this merged?
I just noticed the issue with method_defined? and was about to suggest the same fix.

@fbernier
Copy link
Author

Dunno for the first commit ... Forget about the second one though it's garbage...

@fbernier fbernier reopened this Feb 19, 2013
@csquared
Copy link
Owner

@fbernier @dukedave Sorry i missed this! I've had about 1K notifications that i just marked as all read because I haven't been noticing these important ones.

I just pushed the fix y'all recommended and incremented the version.

Any interest in commit bit?

@csquared csquared closed this Feb 21, 2013
@fbernier
Copy link
Author

Thanks for the merge. I'd be interested in commit if you'd like. I might also spike out a cleaner version of the "timeout option" commit I made eventually.

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.

3 participants