ms-ati / docile

Docile keeps your Ruby DSLs tame and well-behaved
http://ms-ati.github.com/docile/
MIT License
419 stars 34 forks source link

Fix delegation on Ruby 2.7 #52

Closed eregon closed 3 years ago

eregon commented 3 years ago

See https://eregon.me/blog/2019/11/10/the-delegation-challenge-of-ruby27.html#ruby-3-style-delegation and https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0/#a-compatible-delegation for more details.

Addresses https://github.com/ms-ati/docile/pull/45#discussion_r547216011 and properly fixes https://github.com/ms-ati/docile/issues/44.

Also fixes https://github.com/jwt/ruby-jwt/issues/396 and in general many SimpleCov usages.

@taichi-ishitani I wonder, was https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0/ confusing? It intends to say "use ruby2_keywords; (*args, **kwargs)-delegation does not work on Ruby 2.7". But maybe it needs to be clarified further?

codecov-io commented 3 years ago

Codecov Report

Merging #52 (c1080f7) into master (f065fc3) will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master       #52   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            6         6           
  Lines           82        83    +1     
=========================================
+ Hits            82        83    +1     
Impacted Files Coverage Δ
lib/docile/fallback_context_proxy.rb 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f065fc3...c1080f7. Read the comment docs.

taichi-ishitani commented 3 years ago

@eregon , Thank you for fixing a bug caused by my PR. I misread the article.

eregon commented 3 years ago

@taichi-ishitani No worry. Do you have any suggestion how to improve the article to be clearer? It seems at least a few people got confused by it.

ms-ati commented 3 years ago

Thank you for this fix @eregon ! I will make a release that includes this

eregon commented 3 years ago

Thanks!

ms-ati commented 3 years ago

@eregon I've just released v1.3.4 to rubygems, which includes this PR. Thanks again.