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

Allow dd-trace-rb usage in Ruby 3.2 (aka bump maximum version to < 3.3) #1975

Merged
merged 2 commits into from
Apr 13, 2022

Conversation

ivoanjo
Copy link
Member

@ivoanjo ivoanjo commented Apr 12, 2022

A maximum version was initially added in #1495 because we expected the ruby2_keywords method to be removed (see the PR for the discussion).

Now Ruby 3.2.0-preview1 is out and ruby2_keywords are still there, and there's even a recent change for it in ruby/ruby#5684 that is documented as "ruby2_keywords needed in 3.2+".

So for now let's bump the maximum version to < 3.3 to allow the Ruby 3.2 series to be supported and we can keep an eye on the Ruby 3.2 test releases to see if anything changes.

(Otherwise, once Ruby 3.2.0 stable is out, we should probably bump this to 3.4, and so on...)

A maximum version was initially added in
#1495 because we expected
the `ruby2_keywords` method to be removed (see the PR for the
discussion).

Now Ruby 3.2.0-preview1 is out and `ruby2_keywords` are still there,
and there's even a recent change for it in
ruby/ruby#5684 that is documented as
"ruby2_keywords needed in 3.2+".

So for now let's bump the maximum version to < 3.3 to allow the
Ruby 3.2 series to be supported and we can keep an eye on the Ruby 3.2
test releases to see if anything changes.

(Otherwise, once Ruby 3.2.0 stable is out, we should probably bump
this to 3.4, and so on...)
@ivoanjo ivoanjo requested a review from a team April 12, 2022 16:44
@codecov-commenter
Copy link

Codecov Report

Merging #1975 (2e1082a) into master (60ecc8a) will increase coverage by 0.16%.
The diff coverage is 93.68%.

@@            Coverage Diff             @@
##           master    #1975      +/-   ##
==========================================
+ Coverage   97.53%   97.70%   +0.16%     
==========================================
  Files         998     1000       +2     
  Lines       49006    50448    +1442     
==========================================
+ Hits        47800    49289    +1489     
+ Misses       1206     1159      -47     
Impacted Files Coverage Δ
.../datadog/appsec/contrib/rack/request_middleware.rb 34.48% <25.00%> (+10.95%) ⬆️
lib/datadog/appsec/processor.rb 86.20% <86.20%> (ø)
spec/datadog/core/error_spec.rb 94.02% <90.00%> (-0.38%) ⬇️
spec/datadog/appsec/processor_spec.rb 100.00% <100.00%> (ø)
lib/datadog/ci/test.rb 100.00% <0.00%> (ø)
lib/datadog/ci/flush.rb 100.00% <0.00%> (ø)
lib/datadog/profiling.rb 100.00% <0.00%> (ø)
spec/datadog/core_spec.rb 100.00% <0.00%> (ø)
lib/ddtrace/transport/io.rb 100.00% <0.00%> (ø)
spec/datadog/ci/test_spec.rb 100.00% <0.00%> (ø)
... and 211 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

Copy link
Member

@lloeki lloeki left a comment

Choose a reason for hiding this comment

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

Change approved, although I would keep the essence of the first four - lines (albeit without mention of hypothetical version numbers) to keep context about the cause and implications.

@ivoanjo
Copy link
Member Author

ivoanjo commented Apr 13, 2022

Ack! Re-added more context from those first lines in aef2724 .

@ivoanjo ivoanjo merged commit 8a50f1f into master Apr 13, 2022
@ivoanjo ivoanjo deleted the ivoanjo/allow-usage-in-ruby-3-2 branch April 13, 2022 07:52
@github-actions github-actions bot added this to the 1.0.0.beta2 milestone Apr 13, 2022
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.

3 participants