Conversation
| public IRubyObject initialize(IRubyObject[] args, Block block) { | ||
| final Ruby runtime = getRuntime(); | ||
| int argnum = 1; | ||
| IRubyObject sig = runtime.getCurrentContext().nil; |
There was a problem hiding this comment.
its probably better to receive ThreadContext context instead or simply do a runtime.getNil() if the context is not needed anywhere else.
There was a problem hiding this comment.
done. thanks for review!
There was a problem hiding this comment.
@etehtsea @kares whether context is used or not we should always provide it as first argument to @JRubyMethod annotations so it will be future-proofed. We want to make sure these method signatures never need to change for native extension authors.
@etehtsea a second tidbit of info is getRuntime().getCurrentContext() is not super quick which lead to us adding the ability to pass ThreadContext to method bindings. We probably should have made it mandatory in 9k but we didn't :|
d604b64 to
572f7a7
Compare
572f7a7 to
3e4381d
Compare
|
should be 🍏 to go. |
|
@etehtsea There seems to be a number of whitespace changes in your commits. Would it be possible to strip those out? Otherwise I'll need to merge manually and omit them myself. |
3e4381d to
db2490b
Compare
|
@headius done |
db2490b to
6b793be
Compare
Fixes #3954