Update to Celluloid 0.17 #126

Merged
merged 2 commits into from Oct 16, 2015

Conversation

Projects
None yet
3 participants
@donaldpiret
Contributor

donaldpiret commented Sep 1, 2015

Not sure if this is 100% correct. Couldn't really figure out what the CellProxy was doing. Would appreciate if you could have a look.

@brandonhilkert

This comment has been minimized.

Show comment
Hide comment
@brandonhilkert

brandonhilkert Sep 2, 2015

Owner

Hey Donald! Thanks for the this. Give me a little bit to look over and understand what's going on with new Celluloid and I'll get back to you. Thanks again!

Owner

brandonhilkert commented Sep 2, 2015

Hey Donald! Thanks for the this. Give me a little bit to look over and understand what's going on with new Celluloid and I'll get back to you. Thanks again!

@eileencodes

This comment has been minimized.

Show comment
Hide comment
@eileencodes

eileencodes Oct 2, 2015

When you merge this, it needs to go to celluloid 0.17.2, there's apparently a memory leak in anything below 0.17.2 although the repo doesn't confirm this in the changelog. I read about it here mperham/sidekiq#2582

When you merge this, it needs to go to celluloid 0.17.2, there's apparently a memory leak in anything below 0.17.2 although the repo doesn't confirm this in the changelog. I read about it here mperham/sidekiq#2582

@brandonhilkert

This comment has been minimized.

Show comment
Hide comment
@brandonhilkert

brandonhilkert Oct 2, 2015

Owner

I saw a similar Tweet from Mike yesterday. Thanks for the heads up!


Build a Ruby Gem is available!
http://brandonhilkert.com/books/build-a-ruby-gem/?utm_source=gmail-sig&utm_medium=email&utm_campaign=gmail

http://brandonhilkert.com

On Fri, Oct 2, 2015 at 8:58 AM, Eileen M. Uchitelle <
notifications@github.com> wrote:

When you merge this, it needs to go to celluloid 0.17.2, there's
apparently a memory leak in anything below 0.17.2 although the repo doesn't
confirm this in the changelog. I read about it here mperham/sidekiq#2582
mperham/sidekiq#2582


Reply to this email directly or view it on GitHub
#126 (comment)
.

Owner

brandonhilkert commented Oct 2, 2015

I saw a similar Tweet from Mike yesterday. Thanks for the heads up!


Build a Ruby Gem is available!
http://brandonhilkert.com/books/build-a-ruby-gem/?utm_source=gmail-sig&utm_medium=email&utm_campaign=gmail

http://brandonhilkert.com

On Fri, Oct 2, 2015 at 8:58 AM, Eileen M. Uchitelle <
notifications@github.com> wrote:

When you merge this, it needs to go to celluloid 0.17.2, there's
apparently a memory leak in anything below 0.17.2 although the repo doesn't
confirm this in the changelog. I read about it here mperham/sidekiq#2582
mperham/sidekiq#2582


Reply to this email directly or view it on GitHub
#126 (comment)
.

@brandonhilkert

This comment has been minimized.

Show comment
Hide comment
@brandonhilkert

brandonhilkert Oct 14, 2015

Does the use of as mean it won't create the new queue if one is already registered with that name?

Does the use of as mean it won't create the new queue if one is already registered with that name?

This comment has been minimized.

Show comment
Hide comment
@donaldpiret

donaldpiret Oct 14, 2015

Owner

I believe it registers this pool in the same way as Celluloid::Actor[name] = pool did.
If I have some time later I will check wether there's a spec that covers that registered? correctly returns true after the pool is initialised.

Owner

donaldpiret replied Oct 14, 2015

I believe it registers this pool in the same way as Celluloid::Actor[name] = pool did.
If I have some time later I will check wether there's a spec that covers that registered? correctly returns true after the pool is initialised.

This comment has been minimized.

Show comment
Hide comment
@brandonhilkert

brandonhilkert Oct 14, 2015

Cool. That might explain why the specs that check to see if it's registered aren't failing. Would just like to confirm that's the case when you can.

Cool. That might explain why the specs that check to see if it's registered aren't failing. Would just like to confirm that's the case when you can.

@@ -22,5 +22,5 @@ Gem::Specification.new do |gem|
gem.add_development_dependency "rake"
gem.add_development_dependency "pry"
- gem.add_dependency "celluloid", "0.16.0"
+ gem.add_dependency "celluloid", "~> 0.17.1", ">= 0.17.1.2"

This comment has been minimized.

@brandonhilkert

brandonhilkert Oct 16, 2015

Owner

Should we make this "~> 0.17.2"?

@brandonhilkert

brandonhilkert Oct 16, 2015

Owner

Should we make this "~> 0.17.2"?

This comment has been minimized.

@donaldpiret

donaldpiret Oct 19, 2015

Contributor

Yup, thank you for handling that! Sorry I couldn't take care of it earlier. Ended up having a very busy last couple of days.

@donaldpiret

donaldpiret Oct 19, 2015

Contributor

Yup, thank you for handling that! Sorry I couldn't take care of it earlier. Ended up having a very busy last couple of days.

brandonhilkert added a commit that referenced this pull request Oct 16, 2015

@brandonhilkert brandonhilkert merged commit faad882 into brandonhilkert:master Oct 16, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@brandonhilkert

This comment has been minimized.

Show comment
Hide comment
@brandonhilkert

brandonhilkert Oct 20, 2015

Owner

Release version 1.6.0 today as a result of your work. Thanks for the help here!

Owner

brandonhilkert commented Oct 20, 2015

Release version 1.6.0 today as a result of your work. Thanks for the help here!

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