You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by GitBox <gi...@apache.org> on 2021/03/10 08:08:56 UTC
[GitHub] [thrift] Jens-G opened a new pull request #2346: THRIFT-5367 Ruby library crashes when using GC.compact
Jens-G opened a new pull request #2346:
URL: https://github.com/apache/thrift/pull/2346
Client: rb
Patch: Stan Hu
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] stanhu commented on a change in pull request #2346: THRIFT-5367 Ruby library crashes when using GC.compact
Posted by GitBox <gi...@apache.org>.
stanhu commented on a change in pull request #2346:
URL: https://github.com/apache/thrift/pull/2346#discussion_r591193767
##########
File path: lib/rb/thrift.gemspec
##########
@@ -27,7 +27,8 @@ Gem::Specification.new do |s|
s.require_paths = %w[lib ext]
- s.add_development_dependency 'bundler', '~> 1.11'
+ s.add_development_dependency 'bundler', '>= 1.0'
Review comment:
Ruby 2.7 uses Bundler v2 by default, so maybe we should do this?
```suggestion
s.add_development_dependency 'bundler', '>= 1.11'
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jens-G commented on pull request #2346: THRIFT-5367 Ruby library crashes when using GC.compact
Posted by GitBox <gi...@apache.org>.
Jens-G commented on pull request #2346:
URL: https://github.com/apache/thrift/pull/2346#issuecomment-796205233
Create a new one?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jens-G commented on pull request #2346: THRIFT-5367 Ruby library crashes when using GC.compact
Posted by GitBox <gi...@apache.org>.
Jens-G commented on pull request #2346:
URL: https://github.com/apache/thrift/pull/2346#issuecomment-796188141
@stanhu Feel free to improve, it's your patch.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] stanhu commented on pull request #2346: THRIFT-5367 Ruby library crashes when using GC.compact
Posted by GitBox <gi...@apache.org>.
stanhu commented on pull request #2346:
URL: https://github.com/apache/thrift/pull/2346#issuecomment-796227865
Ok, I thought this repository was just a mirror: https://github.com/apache/thrift/pull/2347.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jens-G closed pull request #2346: THRIFT-5367 Ruby library crashes when using GC.compact
Posted by GitBox <gi...@apache.org>.
Jens-G closed pull request #2346:
URL: https://github.com/apache/thrift/pull/2346
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] stanhu commented on a change in pull request #2346: THRIFT-5367 Ruby library crashes when using GC.compact
Posted by GitBox <gi...@apache.org>.
stanhu commented on a change in pull request #2346:
URL: https://github.com/apache/thrift/pull/2346#discussion_r591192428
##########
File path: lib/rb/ext/binary_protocol_accelerated.c
##########
@@ -457,4 +457,5 @@ void Init_binary_protocol_accelerated() {
rb_define_method(bpa_class, "read_set_end", rb_thrift_binary_proto_read_set_end, 0);
rbuf_ivar_id = rb_intern("@rbuf");
+ rb_global_variable(&rbuf_ivar_id);
Review comment:
I suspect this isn't needed because I think `rb_intern` gets marked already.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] stanhu commented on pull request #2346: THRIFT-5367 Ruby library crashes when using GC.compact
Posted by GitBox <gi...@apache.org>.
stanhu commented on pull request #2346:
URL: https://github.com/apache/thrift/pull/2346#issuecomment-796192691
@Jens-G How would you like me to tweak this patch? I don't have write access to this pull request.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] stanhu commented on a change in pull request #2346: THRIFT-5367 Ruby library crashes when using GC.compact
Posted by GitBox <gi...@apache.org>.
stanhu commented on a change in pull request #2346:
URL: https://github.com/apache/thrift/pull/2346#discussion_r591228306
##########
File path: lib/rb/thrift.gemspec
##########
@@ -27,7 +27,8 @@ Gem::Specification.new do |s|
s.require_paths = %w[lib ext]
- s.add_development_dependency 'bundler', '~> 1.11'
+ s.add_development_dependency 'bundler', '>= 1.0'
+ s.add_development_dependency 'srv', '~> 1.0'
Review comment:
We might also want to sort this line.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org