Task/1 refactoring - #2
Merged
Merged
Conversation
mkcosta
force-pushed
the
task/1_refactoring
branch
2 times, most recently
from
July 28, 2026 12:28
9992735 to
b01d1ac
Compare
- sign_charge now delegates to sign_authorize instead of duplicating its body; Stablecoins.eip3009_tokens/eip2612_tokens share a private tokens_supporting helper. - HttpClient#request: retry loop rewritten as an explicit while loop, the triplicated conditional log-field splat collapsed into one log() method, and the Net::HTTP request-class lookup hoisted into a frozen constant. - Eip3009Signature#to_hex now raises on malformed (non-0x-prefixed) r/s instead of silently producing a bad signature. - Added missing frozen_string_literal magic comments for consistency. - Stripped inline implementation comments, section dividers, and docs on private methods; public class/module/constant docs and YARD @param/@return blocks are untouched.
mkcosta
force-pushed
the
task/1_refactoring
branch
2 times, most recently
from
July 28, 2026 13:12
9421a72 to
5d5151b
Compare
mkcosta
force-pushed
the
task/1_refactoring
branch
from
July 28, 2026 13:13
5d5151b to
404de97
Compare
Resolves the five conflicts my error/analytics change (#3) created by landing in main while this branch was open. My fault for merging without checking what was open. Nothing was dropped on either side — the two changes are additive, not competing: - api_error.rb — his grouped @!attribute docs and `freeze`, plus code/title/detail and #hint. Freezing still holds with the extra ivars, they are all set before it. - http_client.rb — entirely his: the body moved to Request, so my edit had nowhere to live here. The fix was RE-APPLIED in request.rb, where those methods now are; taking "his" file wholesale would have silently reverted it, which is the one resolution that does damage: error_code would go back to reading `status` (the family) over `code` (the condition), and error_message would ignore `detail`. request.rb also now passes title: to ApiError. - client.rb — his bare requires (his $LOAD_PATH additions make them work) and his grouped docs and freeze, with analytics wired in and documented alongside the rest. - lib/rail0.rb — his $LOAD_PATH setup and require list, plus error_hints before api_error (ApiError#hint calls Rail0.describe_error). - README.md — his Request/DefaultLogger structure lines, plus the api_error and error_hints descriptions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Found while trying to run this branch's suite: lib/rail0/default_logger.rb requires "logger" at load time, but nothing declares it. `logger` left Ruby's default gems in 4.0, so under bundler on a Ruby this gemspec claims to support (required_ruby_version ">= 3.0", no upper bound) requiring the SDK fails outright: LoadError: cannot load such file -- logger ./lib/rail0/default_logger.rb:3 Declared as a runtime dependency rather than added to the Gemfile's development group, because it is the library that needs it, not the tests. With it, the suite runs: 107 examples, 0 failures. Included in this branch because without it the merge cannot be verified at all; drop it if you would rather pin the Ruby version instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Matches the cleanup this branch made repo-wide — no inline implementation comments, no section dividers, class/module docs and YARD @param/@return kept — to the files that came in from main, which still carried the gateway repo's heavier commenting style. Dropped: the family dividers inside ERROR_HINTS, the rationale paragraphs on ApiError/#hint and the analytics methods, the notes above request.rb's error_code and error_message (this branch had already stripped the originals there), the require note in lib/rail0.rb, and the gemspec paragraph — reduced to the one line that says why the dependency exists. Kept where it earns its place: what each class is for, and every @param/@return. analytics.rb also picks up the conventions the other resources gained here — `require "query"`, `attr_reader :http` with `freeze`, and calling `http` rather than `@http` — so the new resource reads like its siblings rather than like an import. The reasoning those comments carried is not lost: it is in PR #4's description, where the one resolution that could silently revert the fix is spelled out. 107 examples, 0 failures. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
merge: resolve main into task/1_refactoring (error triple + analytics)
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
2 participants
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.
Closes #1