Skip to content

Fix issues found during the connection-reuse review - #8

Merged
jonathan-kosgei merged 8 commits into
mainfrom
fix-noticed-issues
Nov 3, 2026
Merged

jonathan-kosgei merged 8 commits into
mainfrom
fix-noticed-issues

Conversation

@jonathan-kosgei

@jonathan-kosgei jonathan-kosgei commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #7, fixing the items it listed under "Noticed, not changed", plus the release workflow trigger: a push to main used 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.rb is untouched.

Changes

  • lookup(select_field: "city") without an IP requests /city instead of //city: request paths are now built from the non-nil segments.
  • A base_url: with a path prefix (for example https://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. Previously build_uri replaced the whole path and the prefix was lost.
  • The User-Agent header is ipdata-ruby/<version> Ruby/<ruby-version> (<platform>) instead of ruby-ipdata/..., the ipdata-<language>/<version> form the Python client also uses.
  • Release workflow: the push trigger is limited to v* tags (branches: [main] removed), so a merge to main no longer attempts a publish. The workflow_dispatch path (bump, commit, tag, publish) is unchanged.
  • CI workflow: the Ruby magic comment (# frozen_string_literal: true) at the top of ci.yml is removed; the matrix also runs Ruby 3.4 and 4.0 with fail-fast: false, so one version's failure no longer cancels the other jobs; actions/checkout moves from v4 (a Node 20 action, deprecated on the runners) to v5 (Node 24) in both workflows.
  • Test suite on Ruby 4.0: 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, so every stubbing test failed there with NoMethodError. A small with_stub helper in test/test_helper.rb replaces the singleton method for the duration of a block and restores it; the suite now runs under Minitest 5 and 6 without additional gems.
  • Gem metadata: homepage is now https://ipdata.co while source_code_uri stays at this repository, so gem build no longer warns that both keys hold the same URL; CHANGELOG.md now exists (the target of changelog_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 names openssl.
  • test/test_client.rb: the unused local uri in assert_request is removed; ruby -w is clean for the library and the suite.

Not changed

  • README "Development" still says bundle exec rake test: the Rakefile defines that task and it works in a normal setup; CI's ruby -Ilib -Itest -e ... one-liner runs the same suite without Bundler.
  • CHANGELOG.md groups 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 for lookup, select_field and bulk_lookup, and a trailing slash on the base URL.
  • The same one-liner with Minitest 6.0.6 on the load path: 118 runs, 424 assertions, 0 failures, 0 errors. Before the with_stub change 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 (main reported the unused uri).
  • gem build ipdata.gemspec: "Successfully built RubyGem" with no metadata warning (main warned about identical homepage_uri/source_code_uri); the packaged file list includes CHANGELOG.md.
  • CI: run 15 (first push, https://github.com/ipdata/ruby/actions/runs/41639414385) passed tests and gem build on 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 and gem build.

🤖 Generated with Claude Code

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>
@jonathan-kosgei
jonathan-kosgei merged commit 7686b76 into main Nov 3, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants