allowing Data.new to accept StringIO instances #18

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants
@terencehonles
Contributor

terencehonles commented Feb 14, 2013

No description provided.

@ueno

This comment has been minimized.

Show comment
Hide comment
@ueno

ueno Feb 15, 2013

Owner

Hmm, I don't think it's is a good idea to load stringio module unconditionally. Why doesn't it respond to to_io?

Owner

ueno commented Feb 15, 2013

Hmm, I don't think it's is a good idea to load stringio module unconditionally. Why doesn't it respond to to_io?

@terencehonles

This comment has been minimized.

Show comment
Hide comment
@terencehonles

terencehonles Feb 15, 2013

Contributor

I hope you're just speaking aloud... I have no idea why the implementers of StringIO didn't make it respond to to_io or subclass an IO base class, but it doesn't. Ruby is not my preferred language, but If you don't want to take this contribution my employers will continue using my fork of this repository.

Regarding requiring stringio: I did that because I ruby 1.8.7 does not include it by default and it needs to be required. Although we don't use ruby 1.8.7 I figured you might want not to get bug reports from people using ruby 1.8.7

Contributor

terencehonles commented Feb 15, 2013

I hope you're just speaking aloud... I have no idea why the implementers of StringIO didn't make it respond to to_io or subclass an IO base class, but it doesn't. Ruby is not my preferred language, but If you don't want to take this contribution my employers will continue using my fork of this repository.

Regarding requiring stringio: I did that because I ruby 1.8.7 does not include it by default and it needs to be required. Although we don't use ruby 1.8.7 I figured you might want not to get bug reports from people using ruby 1.8.7

@ueno

This comment has been minimized.

Show comment
Hide comment
@ueno

ueno Feb 17, 2013

Owner

How about defined?(StringIO) and object.is_a?(StringIO) instead of requiring it?

Owner

ueno commented Feb 17, 2013

How about defined?(StringIO) and object.is_a?(StringIO) instead of requiring it?

@terencehonles

This comment has been minimized.

Show comment
Hide comment
@terencehonles

terencehonles Feb 17, 2013

Contributor

I guess that's a good alternative. Though I'm not sure it makes much of a
difference. StringIO is a core library it's just not included by default.
On Feb 16, 2013 4:20 PM, "Daiki Ueno" notifications@github.com wrote:

How about defined?(StringIO) and object.is_a?(StringIO) instead of
requiring it?


Reply to this email directly or view it on GitHubhttps://github.com/ueno/ruby-gpgme/pull/18#issuecomment-13677975.

Contributor

terencehonles commented Feb 17, 2013

I guess that's a good alternative. Though I'm not sure it makes much of a
difference. StringIO is a core library it's just not included by default.
On Feb 16, 2013 4:20 PM, "Daiki Ueno" notifications@github.com wrote:

How about defined?(StringIO) and object.is_a?(StringIO) instead of
requiring it?


Reply to this email directly or view it on GitHubhttps://github.com/ueno/ruby-gpgme/pull/18#issuecomment-13677975.

@nobu

This comment has been minimized.

Show comment
Hide comment
@nobu

nobu Feb 20, 2013

Does IOCallback works fine with StringIO, which does not provide event triggers?

nobu commented Feb 20, 2013

Does IOCallback works fine with StringIO, which does not provide event triggers?

@terencehonles

This comment has been minimized.

Show comment
Hide comment
@terencehonles

terencehonles Feb 20, 2013

Contributor

I've been able to encrypt StringIO files and verify their result. (Single/Multiple recipients + armor)

So as far as I can tell it's working

Contributor

terencehonles commented Feb 20, 2013

I've been able to encrypt StringIO files and verify their result. (Single/Multiple recipients + armor)

So as far as I can tell it's working

@ueno

This comment has been minimized.

Show comment
Hide comment
@ueno

ueno Mar 5, 2013

Owner

Fixed in that way: bfede6a

Owner

ueno commented Mar 5, 2013

Fixed in that way: bfede6a

@ueno ueno closed this Mar 5, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment