fix: v2 panel batch — doc pins, actionable deprecations, release idempotency, load-order + dual-install guards - #222
Conversation
…potency, load-order + dual-install guards
Six small items from the as-shipped v2 architecture panel:
1. Docs: pin examples updated 2.0.0.alpha1 -> 2.0.0.beta1 (or latest
2.0.0 prerelease) in README and docs/UPGRADING.md.
2. Deprecation warnings now name the first caller frame outside the
gem's lib dir ("called from file:line"); warn-once semantics
unchanged.
3. Deprecation module header rewritten: it is the live warn-once engine
for the legacy namespace shims, not dormant machinery.
4. release.yml Create tag step is idempotent: skips when the tag exists
at HEAD, fails loudly (never retags) when it exists elsewhere.
5. New subprocess guard: bare require "snap_diff" must never load the
umbrella capybara_screenshot_diff.rb — pins the #208 acyclic require
graph as an executable contract.
6. Dual-install guard in lib/snap_diff.rb: raises DualInstallError when
both capybara-screenshot-diff and snap_diff-capybara gems are
activated (identical files, silent version skew otherwise).
Reviewer's GuideUpdates docs for the 2.0 beta prerelease, makes deprecation warnings more actionable, strengthens load-order and dual-install safeguards in the core SnapDiff entrypoint, and makes the GitHub release tagging workflow idempotent and safer to rerun. Sequence diagram for actionable legacy deprecation warningssequenceDiagram
participant UserCode
participant LegacyShims
participant Deprecation
participant Kernel
UserCode->>LegacyShims: const_missing
LegacyShims->>Deprecation: warn(subject, replacement, category)
Deprecation->>Deprecation: caller_locations(1)
Deprecation->>Deprecation: origin_for(locations)
Deprecation->>Kernel: warn(message with file:line)
Kernel-->>UserCode: deprecation warning
Flow diagram for safe idempotent release taggingflowchart TD
A[Create tag] --> B[Fetch remote tag ref]
B --> C{Tag exists?}
C -->|No| D[git tag -a and git push]
C -->|Yes| E{Tag points to HEAD?}
E -->|Yes| F[Skip tag creation]
E -->|No| G[Fail workflow; refuse to retag]
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe PR adds a dual-gem loading guard, caller locations to deprecation warnings, idempotent release tag creation, and updated 2.0 prerelease documentation. ChangesRuntime safeguards
Release and documentation
Estimated code review effort: 3 (Moderate) | ~20 minutes ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Hey - I've reviewed your changes and they look great!
Sourcery assessment
Needs a human reviewer. A faulty release decision can create or accept the wrong version tag and proceed with publishing, leaving an externally visible release artifact that reverting the workflow does not remove. The dual-install guard can also turn application startup into a failure for environments with both gem specs activated, though that part is reversible by reverting.
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Six small items from the as-shipped v2 architecture panel, each with its own evidence.
1. Doc version pins (README.md, docs/UPGRADING.md)
All
2.0.0.alpha1pin examples updated to2.0.0.beta1, with "(or the latest 2.0.0 prerelease)" where it reads naturally.Evidence:
grep -rn alpha1 README.md docs/→ no matches (CHANGELOG untouched by design).2. Actionable deprecation warnings (lib/snap_diff/deprecation.rb)
The warning now appends
(called from <file:line>)— the first caller frame outside the gem's lib dir (same filtering idea asBacktraceFilter). Warn-once semantics unchanged.Evidence (TDD): new test "warning names the caller's file and line" was red first (
Expected /called from .../ to match "...instead.\n"), green after the implementation. End-to-end probe through the legacy shim path:(gem-internal legacy_shims frames correctly skipped).
3. Stale "dormant" comment (lib/snap_diff/deprecation.rb)
Header rewritten: the module is the live warn-once engine for the legacy-namespace shims (
snap_diff/legacy_shimsroutes everyconst_missinghit through it), not dormant machinery.4. release.yml tag idempotency
"Create tag" now fetches the tag ref (checkout doesn't fetch tags), skips when the tag already exists at HEAD, and fails loudly — never retags — when it exists at a different commit.
Evidence: shell simulation of all three cases: fresh run →
tagged; re-run →skip: exists at HEAD; moved HEAD →FAIL LOUD: exists at <sha>, HEAD <sha>with exit 1.5. Load-order guard (test/unit/snap_diff_test.rb)
New subprocess probe: under bare
require "snap_diff",lib/capybara_screenshot_diff.rbmust be absent from$LOADED_FEATURES. This turns the #208 acyclic-require-graph fix (the deadlock/partial-initialization bug class) from discipline into an executable contract — a panel probe previously reintroduced the cycle and the suite stayed green.Gate evidence: temporarily adding
require "capybara_screenshot_diff"tolib/snap_diff/config.rb→ guard red (umbrella loaded via: .../lib/capybara_screenshot_diff.rb); revert → green.6. Dual-install conflict guard (lib/snap_diff.rb)
Both gem names ship identical files; with both activated, requires resolve silently from whichever activated first and version skew is undetectable. The shared entry now raises
SnapDiff::DualInstallError(naming both gems, instructing to remove one) whenGem.loaded_specscontains bothcapybara-screenshot-diffandsnap_diff-capybara. Single-gem installs, local dev from source (neither spec loaded), and the test suite are unaffected.Evidence (TDD): tests red first (
NameError: uninitialized constant SnapDiff::DualInstallError), green after; pass-cases cover each single gem and the empty (source-checkout) case.Test evidence
rake test:unit→ 488 runs, 1417 assertions, 0 failuresrake test:unit→ 492 runs, 1427 assertions, 0 failures, 0 errorsrake test→ 525 runs, 1472 assertions, 0 failures, 0 errors, 6 skips (pre-existing driver skips)standardrbon all touched Ruby files: no offenses🤖 Generated with Claude Code
Summary by Sourcery
Harden the v2 prerelease transition with safer release tagging, clearer deprecations, installation conflict detection, load-order safeguards, and current documentation.
Bug Fixes:
Enhancements:
CI:
Documentation:
Tests:
Summary by CodeRabbit
Bug Fixes
Documentation