Add ruby 3.4 support #808
Add ruby 3.4 support #808
Conversation
lib/http/headers.rb
Outdated
| # @return [String] | ||
| def inspect | ||
| "#<#{self.class} #{to_h.inspect}>" | ||
| "#<#{self.class}>" |
There was a problem hiding this comment.
What's with these seemingly unrelated changes?
There was a problem hiding this comment.
LOL, that was my question as well when I saw changes to headers and response in SOCKS5 PR. Ruby 3.4 changed the output of Hash#inspect:
> chruby-exec ruby-3.4 -- ruby -e 'puts({ a: 42, "b" => 161 }.inspect)'
{a: 42, "b" => 161}
> chruby-exec ruby-3.3 -- ruby -e 'puts({ a: 42, "b" => 161 }.inspect)'
{:a=>42, "b"=>161}
> chruby-exec ruby-3.2 -- ruby -e 'puts({ a: 42, "b" => 161 }.inspect)'
{:a=>42, "b"=>161}
So we could either switch to JSON.dump(...), or just remove those. And I found myself being mostly annoyed with the inspect output of a real-world Response and Headers in the console:
Before this PR
After this PR
There was a problem hiding this comment.
@tarcieri any objections against merging this?
There was a problem hiding this comment.
I would personally prefer such changes be considered in a separate PR but don’t consider it a blocker
There was a problem hiding this comment.
Hm. I can extract this change in a "prepare for ruby-3.4" PR.
There was a problem hiding this comment.
These changes don’t seem directly related to the Ruby 3.4 upgrade, even if it changes the inspect output
There was a problem hiding this comment.
I agree, thus they are in a separate commit ;D
There was a problem hiding this comment.
The repo is on squash-and-merge. You can shut that off I guess and just merge the two commits, but really it seems like the goal of those changes is to change the inspect output format independent of the Ruby version.
Anyway, merge if you want I guess.
There was a problem hiding this comment.
I've already separated the PRs LOL.
On the squash&merge topic:
Although I was actually thinking on git merge origin add-ruby-3-4-support:main as GH allows that when the PR is approved ;))
> chruby ruby-3.4
> ruby -e 'puts({ a: 42, "b" => 161 }.inspect)'
{a: 42, "b" => 161}
> chruby ruby-3.3
> ruby -e 'puts({ a: 42, "b" => 161 }.inspect)'
{:a=>42, "b"=>161}
> chruby ruby-3.2
> ruby -e 'puts({ a: 42, "b" => 161 }.inspect)'
{:a=>42, "b"=>161}
See: httprb/http#808 (comment)
> chruby ruby-3.4
> ruby -e 'puts({ a: 42, "b" => 161 }.inspect)'
{a: 42, "b" => 161}
> chruby ruby-3.3
> ruby -e 'puts({ a: 42, "b" => 161 }.inspect)'
{:a=>42, "b"=>161}
> chruby ruby-3.2
> ruby -e 'puts({ a: 42, "b" => 161 }.inspect)'
{:a=>42, "b"=>161}
See: httprb/http#808 (comment)
* Dumping all headers in `Headers#inspect` mostly useless * Same about response, the only header part that is meaningfull there is MIME type More importantly, Ruby-3.4 changed the inspect output, and we will either need to have if/else in specs, or modify the output to become ruby version-agnostic (i.e., not using Hash#inspect). Blocks: #808
* Dumping all headers in `Headers#inspect` mostly useless * Same about response, the only header part that is meaningfull there is MIME type More importantly, Ruby-3.4 changed the inspect output, and we will either need to have if/else in specs, or modify the output to become ruby version-agnostic (i.e., not using Hash#inspect). Blocks: #808
ixti
force-pushed
the
add-ruby-3-4-support
branch
from
5432fb0 to
f66c9cc
Compare
|
@tarcieri merged as three separate PRs. |
Hash#inspectoutput changed in Ruby-3.4 thus specs were failing. But even without that I don't think dumping all headers inResponse#inspectwas pointless, same aboutHeaders#inspect.