-
Notifications
You must be signed in to change notification settings - Fork 114
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
Add ruby 3.3 support and update test suite to use a local server #216
Conversation
Yep, this is awesome. +1 |
With Ruby 3.3 I observed weird behavior in the encoding of diacritic (special chars) like:
I needed to add
|
This is perfect. If it looks good, lets get it merged. :-) |
@afromankenobi Thank you for your hard work! 👍 🎉 |
@abrom can you take a look at this pls? |
Soz @afromankenobi i've been on holidays 😉 Taking a look over it now |
Revert some of the spec simplifications Fix header count bug in `/headers` endpoint
lib/grover/version.rb
Outdated
@@ -1,5 +1,5 @@ | |||
# frozen_string_literal: true | |||
|
|||
class Grover | |||
VERSION = '1.1.5' | |||
VERSION = '1.2.0' |
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.
i would recommend you don't change the version in a PR.. you're assuming to know the version change policy of the project.
end | ||
end | ||
|
||
get '/' do |
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.
nice one 👍 Fixing this has been on my list for months.. this looks like a nice long term solution
|
||
get '/cookie_renderer' do | ||
cookie_info = request.cookies.map.with_index(1) do |(name, value), i| | ||
"#{i}. #{name} #{value}" |
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.
not quite the same rendering as the original, but it'll do (the original used a table)
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.
Since I wasn't able to get in touch with the original, I had to resort to improvisation.
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.
No worries.. I wrote it, I've just "misplaced" the source.. What you did is fine 😄
Released in v1.1.6 |
Side note: I tried this before but hit problems with the grover gem. Those problems are resolved now with grover 1.1.6, see Studiosity/grover#216 .
This PR is my attempt to fix Grover's use of Ruby 3.3 and remove the dependency on the external resources the test suite used. It basically:
I hope this is helpful :).
Related to #212 and #213