-
-
Notifications
You must be signed in to change notification settings - Fork 85
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
Implement ByteBuffers #95
Conversation
* @return | ||
*/ | ||
@JRubyMethod | ||
public IRubyObject putAByte(ThreadContext context, IRubyObject aByte){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure this is useful versus always acting on multiple bytes
Some suggestions on next steps:
|
Thanks Tony, I will follow those suggestions. 👍 |
@tarcieri Since " this API should be used for adding strings to ByteBuffers". I guess I can avoid the methods such as putAbyte and putInt(), ... and just think about the Strings. In the java API they support all sorts of data types. That is why I intended to include a method such as putAbyte. Anyway I would have a generalize put method ("<<") that can handle Strings, Numbers and other byteArrays without a problem :) . I guess that would be better. |
@UpeksheJay is also me 😃 |
- ruby-head | ||
- jruby | ||
- jruby-1.7 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this line and it should fix the build. JRuby 1.7 is very old and we probably don't need to support it anymore.
10da2c6
to
4754d51
Compare
@UpeksheJay I just squashed your commits and also did some formatting cleanups on the C code (hard tabs -> 4 spaces, and some cleanups to brace formatting for If this passes the tests and looks good to you, you'll want to do the following on your local copy (provided your local branch is also named
That should blow away your local branch and use the upstream one from GitHub. If you do
Let me know if that all sounds good. |
public IRubyObject initialize(ThreadContext context, IRubyObject value, IRubyObject offset, IRubyObject length) { | ||
Ruby ruby = context.getRuntime(); | ||
if (value == ruby.getNil()) | ||
throw new IllegalArgumentException(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you probably want to have the exception type consistent across implementations, currently it is a different kind (based on backend used) : this could be improved to do rb_raise(rb_eTypeError, "not a valid input")
like in MRI : throw new runtime.newTypeError("not a valid input")
Hi Tony, I will add new commit to bytebuffers branch with fixes for the issues pointed out by Kares. BTW, thnks Kares for the reviews 👍 |
Signed-off-by: UpeksheJay <[email protected]>
Thanks @Upekshe for your contributions! |
This branch contains a fairly large new I/O buffer subsystem: #95 This could prevent complications for Rails 5, so let's ship it as a completely new major version of nio4r.
This is @UpeksheJay's GSoC work