Repository navigation
Fix issues found during the connection-reuse review - #8
Merged
Merged
Conversation
ci.yml started with "# frozen_string_literal: true", a Ruby directive that means nothing in YAML. The matrix now also covers Ruby 3.4 and 4.0 next to 3.0 to 3.3, and actions/checkout moves to v5, which runs on Node 24; v4 is a Node 20 action that the runners only run with a deprecation warning. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The push trigger of the release workflow listed branches: [main] next to tags: ["v*"], so every push to main, including a plain merge, ran the release job and tried to publish the current version again, which RubyGems rejects. The push trigger is now limited to v* tags. The manual workflow_dispatch path (bump, commit, tag, publish) is unchanged apart from actions/checkout moving to v5 (Node 24). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
lookup(select_field: "city") without an IP built the path "//city", because the nil IP was interpolated as an empty segment, and build_uri replaced the whole path of base_url:, so a base URL with a path prefix (for example https://example.com/ipdata) lost that prefix. Request paths are now assembled from the non-nil segments under the base URL's own path, and a trailing slash on the base URL is tolerated. Tests cover the single-field lookup of the caller's own IP and prefixed base URLs for lookup, select_field and bulk_lookup. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The header read "ruby-ipdata/<version> Ruby/<ruby-version> (<platform>)". The product token now has the ipdata-<language>/<version> form that the ipdata Python client also uses; the Ruby version and platform suffix is unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The helper parsed the captured request path into a local that nothing read, which ruby -w reported as an assigned but unused variable. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The gemspec pointed changelog_uri at a CHANGELOG.md that did not exist. The file now exists, lists the changes since the 0.1.5 release under "Unreleased" and is packaged with the gem. homepage_uri and source_code_uri were the same URL, which gem build warned about since rubygems.org shows only one of them; the homepage is now https://ipdata.co while source_code_uri keeps pointing at the repository. The stdlib comment also names openssl, which the client requires. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The suite stubbed Net::HTTP.new and Process.pid with Object#stub from minitest/mock, which it never required: Minitest 5's minitest/autorun loaded it implicitly. Minitest 6, which Ruby 4.0 bundles, no longer ships minitest/mock (it moved to the separate minitest-mock gem), so every stubbing test failed there with NoMethodError. A small with_stub helper in test_helper.rb now replaces the singleton method for the duration of a block and restores it afterwards; the call sites change only in the helper name. Verified with Minitest 5.26 and 6.0. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
With the default fail-fast strategy one failing matrix job cancels the rest, which hides whether a failure is specific to that Ruby version. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #7, fixing the items it listed under "Noticed, not changed", plus the release workflow trigger: a push to
mainused to run the RubyGems publish job, so the merge of #7 attempted to publish 0.1.5 again and failed with "Repushing of gem versions is not allowed" (https://github.com/ipdata/ruby/actions/runs/41635184527). Eight commits, one per theme. No version change;lib/ipdata/version.rbis untouched.Changes
lookup(select_field: "city")without an IP requests/cityinstead of//city: request paths are now built from the non-nil segments.base_url:with a path prefix (for examplehttps://example.com/ipdata) keeps the prefix in front of every request path (/ipdata/8.8.8.8,/ipdata/8.8.8.8/city,/ipdata/bulk, and/ipdata/for the caller's own IP); a trailing slash on the base URL is tolerated. Previouslybuild_urireplaced the whole path and the prefix was lost.User-Agentheader isipdata-ruby/<version> Ruby/<ruby-version> (<platform>)instead ofruby-ipdata/..., theipdata-<language>/<version>form the Python client also uses.pushtrigger is limited tov*tags (branches: [main]removed), so a merge tomainno longer attempts a publish. Theworkflow_dispatchpath (bump, commit, tag, publish) is unchanged.# frozen_string_literal: true) at the top ofci.ymlis removed; the matrix also runs Ruby 3.4 and 4.0 withfail-fast: false, so one version's failure no longer cancels the other jobs;actions/checkoutmoves from v4 (a Node 20 action, deprecated on the runners) to v5 (Node 24) in both workflows.Net::HTTP.newandProcess.pidwithObject#stubfromminitest/mock, which it never required (Minitest 5'sminitest/autorunloaded it implicitly). Minitest 6, which Ruby 4.0 bundles, no longer shipsminitest/mock, so every stubbing test failed there withNoMethodError. A smallwith_stubhelper intest/test_helper.rbreplaces the singleton method for the duration of a block and restores it; the suite now runs under Minitest 5 and 6 without additional gems.homepageis nowhttps://ipdata.cowhilesource_code_uristays at this repository, sogem buildno longer warns that both keys hold the same URL;CHANGELOG.mdnow exists (the target ofchangelog_uri), lists everything since 0.1.5 under "Unreleased" in the format of the PHP client's changelog, and is packaged with the gem. The stdlib comment in the gemspec also namesopenssl.test/test_client.rb: the unused localuriinassert_requestis removed;ruby -wis clean for the library and the suite.Not changed
bundle exec rake test: the Rakefile defines that task and it works in a normal setup; CI'sruby -Ilib -Itest -e ...one-liner runs the same suite without Bundler.CHANGELOG.mdgroups the connection-reuse and new-field changes from Reuse HTTPS connections, add new response fields #7 together with the changes here under "Unreleased", because nothing has been tagged since 0.1.5; move them under the version heading when that release is cut.Verification
Ruby 3.3.6, run after each commit and again before each push:
ruby -Ilib -Itest -e "Dir['test/test_*.rb'].each { |f| require File.expand_path(f) }": 118 runs, 424 assertions, 0 failures, 0 errors, 0 skips (main: 115 runs, 400 assertions). Three tests added: single-field lookup without an IP, a prefixed base URL forlookup,select_fieldandbulk_lookup, and a trailing slash on the base URL.with_stubchange it reported 42 errors (undefined method 'stub' for class Net::HTTP), the same failure the first CI run showed on Ruby 4.0.ruby -w -Ilib -Itest -e "Dir['test/test_*.rb'].each { |f| require File.expand_path(f) }": no warnings with Minitest 5.26.2 or 6.0.6 (mainreported the unuseduri).gem build ipdata.gemspec: "Successfully built RubyGem" with no metadata warning (mainwarned about identicalhomepage_uri/source_code_uri); the packaged file list includesCHANGELOG.md.gem buildon Ruby 3.0 to 3.4 and failed only on Ruby 4.0 for the Minitest 6 reason above; fail-fast then cancelled the passing jobs. Run 16 on the current head b037c14 (https://github.com/ipdata/ruby/actions/runs/41640391401): success, all six matrix jobs (Ruby 3.0, 3.1, 3.2, 3.3, 3.4, 4.0) passed the tests andgem build.🤖 Generated with Claude Code