-
Notifications
You must be signed in to change notification settings - Fork 252
Add explicit dependency to 'cgi' for ruby 4.0 support #560
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds an explicit dependency on the cgi gem to ensure compatibility with Ruby 4.0, which has removed cgi from its default gems (except for cgi/escape). Since the codebase uses CGI::Cookie.parse (which is NOT part of cgi/escape) and CGI.unescape in lib/secure_headers/headers/cookie.rb, this dependency is necessary for the gem to function properly with Ruby 4.0.
Key Changes:
- Added
cgigem as a runtime dependency with version constraint~> 0.5.0
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Updates our rubocop config to prevent lint errors from blocking CI. Once #560 is unblocked we can focus on Ruby 4.0 support and cleaning up the linting rules that were disabled in this PR. ## All PRs: * [x] Has tests * [ ] Documentation updated
rei-moo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we add Ruby 4.0 support to our CI matrix here as well?
We'll need to update setup-ruby to a version that supports 4.0
|
@rei-moo I added 4.0.0-preview2 as well as 3.4. We can bump it to 4.0.0 GA when its available. |
|
@rei-moo Hm. Looks like the |
|
@vcsjones I believe you can already specify Hm. Bummer about losing 2.6 support in setup-ruby. I'll look into trying to make sure that we still support older versions for as long as is reasonable. Dropping from CI makes sense for now. 👍 |
rei-moo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Thank you 👏
Ruby 4.0 (currently preview 2) has removed the default dependency on
CGI:We use
CGI::Cookiesecure_headers/lib/secure_headers/headers/cookie.rb
Line 63 in 8b1029c
So we should take an explicit dependency on the
cgigem.