Skip to content

Support rack 3#1884

Merged
sl0thentr0py merged 12 commits intomasterfrom
neel/rack-3
Sep 13, 2022
Merged

Support rack 3#1884
sl0thentr0py merged 12 commits intomasterfrom
neel/rack-3

Conversation

@sl0thentr0py
Copy link
Copy Markdown
Member

@sl0thentr0py sl0thentr0py commented Sep 7, 2022

  • Fix is_server_protocol? check for rack 3 support
  • fix rack lint spec since uppercase no longer allowed in response headers
  • expand CI matrix to test on rack 2 & 3
  • remove bundler: 1 pin in actions
  • sqlite3 1.5.0 has pre-built binaries from ruby 2.6 onwards so use force_ruby_platform for 2.4 and 2.5 for delayed_job

* fix rack lint spec since uppercase no longer allowed in response
  headers
* expand CI matrix to test on rack 2 & 3
@sl0thentr0py sl0thentr0py requested a review from st0012 September 7, 2022 13:19
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Sep 8, 2022

Codecov Report

Base: 98.42% // Head: 98.40% // Decreases project coverage by -0.02% ⚠️

Coverage data is based on head (4fe3adf) compared to base (353a813).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1884      +/-   ##
==========================================
- Coverage   98.42%   98.40%   -0.03%     
==========================================
  Files         148      148              
  Lines        8884     8886       +2     
==========================================
  Hits         8744     8744              
- Misses        140      142       +2     
Impacted Files Coverage Δ
sentry-ruby/lib/sentry/interfaces/request.rb 93.93% <100.00%> (-1.38%) ⬇️
...y-ruby/spec/sentry/rack/capture_exceptions_spec.rb 100.00% <100.00%> (ø)
sentry-ruby/lib/sentry/breadcrumb.rb 96.29% <0.00%> (-3.71%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@sl0thentr0py
Copy link
Copy Markdown
Member Author

sl0thentr0py commented Sep 8, 2022

ughh now sqlite is failing 😭
edit: fixed by setting force_ruby_platform on 2.4 and 2.5

@sl0thentr0py
Copy link
Copy Markdown
Member Author

@st0012 also one more thing I noticed, we have bundler: 1 in all the github actions, any reason we're pinning to such an old bundler version?

@st0012
Copy link
Copy Markdown
Contributor

st0012 commented Sep 12, 2022

@sl0thentr0py I suspect it may be related to Ruby 2.4 support? We can try removing that config though.

@st0012 st0012 added this to the 5.5.0 milestone Sep 12, 2022
Copy link
Copy Markdown
Contributor

@st0012 st0012 left a comment

Choose a reason for hiding this comment

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

All look good, just have a small comment. Great job 🙌

# NOTE: This will be removed in version 3.0+
def is_server_protocol?(key, value, protocol_version)
rack_version = Gem::Version.new(::Rack.release)
return false if rack_version >= Gem::Version.new("3.0")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@sl0thentr0py consider we have this change, it probably still needs a changelog entry.

@sl0thentr0py
Copy link
Copy Markdown
Member Author

@st0012 can you approve if ok? (we added a required check for approval now for compliance reasons, so need it to merge)

@sl0thentr0py sl0thentr0py merged commit 978cc48 into master Sep 13, 2022
@sl0thentr0py sl0thentr0py deleted the neel/rack-3 branch September 13, 2022 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants