Skip to content
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

Add block support to rate queue #17

Merged
merged 3 commits into from
Aug 30, 2021

Conversation

kvokka
Copy link
Contributor

@kvokka kvokka commented Oct 18, 2020

Thank you for great gem!

  • Add blocks support for RateQueue class, which allow to use this gem with custom metrics.
  • Fix Ruby 2.7 warnings

Also:
Close #16

@kvokka kvokka force-pushed the add-block-support-to-rate-queue branch 2 times, most recently from 182be19 to c5eeb06 Compare October 19, 2020 00:51
@kvokka
Copy link
Contributor Author

kvokka commented Jun 20, 2021

@sbfaulkner can you please take a look? Thank you

@sbfaulkner
Copy link
Contributor

thanks @kvokka -- if you pull the upstream and rebase your changes on that I can look at merging this for a v2.1.0

@kvokka kvokka force-pushed the add-block-support-to-rate-queue branch from c5eeb06 to cf256da Compare June 22, 2021 23:03
@kvokka
Copy link
Contributor Author

kvokka commented Jun 22, 2021

@sbfaulkner Thank you for the update. I rebased the changes on the current master.

Also, FYI, rubocop can not execute, cos https://shopify.github.io/ruby-style-guide/rubocop.yml is not avaliable.


mixin = Module.new do
if RUBY_VERSION < "2.7"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since the gem require ruby 2.6+ we can simplify this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we? (ie. since 2.6 is still < 2.7)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think my testing showed that wouldn't work on 2.6

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ruby 2.7 has deprecated automatic conversion from a hash to keyword arguments.
We do not need to worry about 2.6 here, cos the syntax is valid for 2.0+

Details

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah... I get that... I just thought I'd tested on 2.6 and needed to handle differently... I may be misremembering :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Np, just for sure i installed Ruby 2.6.3 locally and checked - everything is ok.

But in the process of this check was revealed that travisci just does not work on the repo. Just letting you know

@kvokka kvokka requested a review from sbfaulkner July 20, 2021 17:57
@kvokka
Copy link
Contributor Author

kvokka commented Jul 20, 2021

Sorry @sbfaulkner , forgot to re-request the review.

@kvokka
Copy link
Contributor Author

kvokka commented Aug 30, 2021

@sbfaulkner ping

@sbfaulkner sbfaulkner merged commit 0de8da6 into Shopify:master Aug 30, 2021
@shopify-shipit shopify-shipit bot temporarily deployed to production August 30, 2021 13:23 Inactive
@sbfaulkner
Copy link
Contributor

@kvokka https://rubygems.org/gems/ruby-limiter/versions/2.1.0
🥂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants