docs: the legacy API is removed in 2.1, not 3.0 - #247
Merged
Conversation
2.0 is the transitional release; 2.1 deletes the legacy namespace trees. The migration notice, its pinning test, the reverse gate's constant and the deletion test's name all still said 3.0 -- text that ships to users in 2.0. The notice test now asserts the whole clause rather than the version alone, so a wrong deprecation WINDOW fails it too, not just a wrong removal version.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
Warning Review limit reached
Next review available in: 37 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (20)
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 |
Contributor
Reviewer's GuideAligns all deprecation copy, docs, and tests with the fact that the legacy v1 API is removed in 2.1 (not 3.0), and makes the deletion tests use durable, version-agnostic naming while tightening the migration notice assertion. File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
This was referenced Aug 23, 2026
pftg
added a commit
that referenced
this pull request
Aug 24, 2026
* docs: 2.0.0 release readiness Audit of everything a 2.0.0 final would ship, and the fixes that did not need lib/ changes. CHANGELOG - A v2.0.0 entry written for someone upgrading from 1.15.1, not a diff of the betas. What to change (the version), what they will see (exact warning text), the five things that can actually break, and what 2.1 removes. Every claim verified against a real install; the beta sections stay as history. Version consistency - README, docs/UPGRADING.md: no more "beta"/"alpha"/"experiment" framing and no beta pins. Gemfile examples say `~> 2.0`. - Gem name: `capybara-screenshot-diff` is the one we tell people to install; `snap_diff-capybara` is a reserved identical mirror. Stated once in the README with the dual-install consequence, applied everywhere else. - Stale "3.0" references in the Rakefile and docs/architecture.md are now 2.1 (#247 fixed the user docs and missed these). Corrections to claims that were not true - docs/drivers.md promised that everything 2.1 removes "warns once per process naming 2.1". `driver: :auto` is silent whenever ruby-vips is present, and the `driver:` setting itself never warns at all even though 2.1 deletes it (`NoMethodError`). Both are now written down as silent, in drivers.md and UPGRADING.md, since a note is the only notice they can get. - README called ruby-vips "Optional". With neither ruby-vips nor chunky_png installed, comparisons raise `Wrong adapter nil. Available adapters: []`. Says so now. - Setup examples no longer teach `driver: :vips`, a line users have to delete for 2.1. Gem hygiene - gemspec: summary/description that describe what the gem does, the rubygems metadata links (source, changelog, bug tracker, docs), and docs/docker-testing.md dropped from the package (it documents bin/dtest, which is not packaged). Dead bindir/executables removed -- the allow-list never matched exe/. - README's links to CONTRIBUTING.md and docker-testing.md are absolute, so they resolve from inside the gem too. - test/unit/gemspec_packaging_test.rb pins the packaged file list: both Bundler.require entry files present (this broke twice), consumer docs in, contributor docs and build files out, capybara the only runtime dependency. Verified it fails when an entry file is unpackaged. - *.gem is gitignored. Release process - The GitHub Release body linked to blob/main on a repo whose default branch is master -- 404 on every release so far. Links to the tag now. - docs/RELEASE_PREP.md was a stale v1.15.1 checklist. It is now a runbook for how releases actually happen: what the workflow does step by step, the trusted-publisher prerequisite for BOTH gem names, prereleases, post-release verification, and what to do when a run fails halfway. - CONTRIBUTING.md pointed at the wrong version.rb and recommended `rake release`, which publishes only one of the two gem names. Verified with real installs on ruby 4.0.6: 1.15.1 -> this master via path:, a canonical-names setup, `Bundler.require` under each gem name from the built .gem, and the dual-install guard with both gems installed. No lib/ changes. Version not bumped. * docs: SnapDiff::Error is the base class for errors the gem defines, not every error it raises docs/snapdiff.md's object map said "Base class for every error this gem raises". It is not: a missing image backend raises a bare RuntimeError ("Wrong adapter nil. Available adapters: []", reproduced on a bundle with neither ruby-vips nor chunky_png) and StableScreenshoter raises ArgumentError. Verified the four defined errors -- ExpectationNotMet, UnstableImage, WindowSizeMismatchError, DualInstallError -- do all inherit SnapDiff::Error, so the useful half of the promise holds and is now the one being made. * docs: tag protection, not branch protection, is what gates the release tag push Branch protection rules do not govern tag pushes; tag protection rules (or rulesets) do. The runbook prerequisite now names the right control. * docs: the per-screenshot driver: override dies quietly, the config setting dies loudly Both were lumped together as "raises NoMethodError on 2.1". Only the config setting does. Per-screenshot options are a free-form hash, so on 2.1 `screenshot "index", driver: :vips` is inert and nothing tells you the line is dead -- #249's own upgrade note spells out the split. Grep-for-it advice added, since that is the only signal a user gets. * docs: pin versions, and stop telling people to delete baselines Second pass, from customer-persona findings. Each verified here before acting; two of the four reported items turned out to be artifacts of the PUBLISHED beta3 rather than of master, and are handled as such. Baselines (the oldest bug in the tracker: #5 and #6 in 2018, #133 in 2024) - README told users to "delete the baseline and re-run" in two places. It cannot work. `Vcs.checkout_vcs` (lib/snap_diff/vcs.rb:24) resolves every baseline with `git show HEAD:<path>`, and `ScreenshotMatcher#check_base_screenshot` calls it before `need_to_compare?` tests `base_path.exist?` -- so a committed baseline is fetched from HEAD no matter what the working tree says, and `rm` changes nothing. - New first-class "Accepting an intentional change" section: the mechanism, the commit that actually accepts it, and the surprising part -- staging is not enough, so no local run goes green until you commit. The FAQ answer now says the same thing instead of the opposite. - Deliberately does NOT document RECORD_SCREENSHOTS. It is printed by our own error message (screenshot_matcher.rb:73) but read nowhere in lib/; a separate lane is implementing it, and it should be documented once it works, not before. Version pinning - `gem "snap_diff-capybara"` unpinned installs 0.0.1 -- a placeholder whose entire payload is one README and zero Ruby files (verified by fetching and unpacking it), so the user gets an immediate LoadError. And unpinned `gem "capybara-screenshot-diff"` resolves to 1.15.1, not to the 2.0 the surrounding prose is selling. Every install instruction now pins, and the README says plainly that the mirror name is not the one to reach for. CHANGELOG, all verified - Failure messages leaked a libvips pointer struct via the comparison metadata; `to_h` excludes `diff_mask` since #234, which landed after the beta3 tag, so 2.0.0 final is the fix. - Known limitation: fork-parallel runs write no HTML report. Workers accumulate assertions per process; the report is written from `Minitest.after_run` in the parent (integrations/minitest.rb:69), which never sees them. Artifacts and pass/fail are unaffected. - A note for anyone sitting on a prerelease: beta3's deprecation channel was incomplete, so its silence is not evidence of being migrated. Constants - `Capybara::Screenshot::Os` -> `SnapDiff::Os` was in no rename table. Gemspec - rubygems_mfa_required. The four URI fields were added in the first commit. * docs: stop teaching two commands that do not work Two customer personas independently followed the docs and got a green bar on a page they had deliberately broken. `rake test` does not run `test/system/` in a Rails app. The Quick Start told users to run it, so step 1 produced `0 runs` and no baselines -- which reads as a pass. The example is a Rails system test; the command now matches it, with a callout, because "0 runs" is the single easiest way to believe visual testing is working when nothing is running. `RECORD_SCREENSHOTS=1` appeared in three user-facing docs for a feature that has never existed in `lib/` -- it is this repository's own test-suite convention, read by `test/test_helper.rb`. The user-facing copies are replaced with the flow that actually works: run the suite, which rewrites every changed baseline in place, then `git add` and commit. The contributor page keeps it and now says plainly that it is not a library feature. * docs: make the Quick Start something a new user can actually run A reviewer built a stock Rails 8.1 app, ran the documented Quick Start end to end, and none of it worked. Fixes, each verified against a scratch app or against lib/: - `gem "capybara-screenshot-diff", "~> 2.0"` does not resolve. rubygems has 1.15.1 and 2.0.0.alpha1..beta3 and no final 2.x, and Bundler never picks a prerelease from a plain requirement, so `bundle install` fails with `Could not find gem 'capybara-screenshot-diff (~> 2.0)'`. All five install snippets now pin `2.0.0.beta3` and say why. RELEASE_PREP gains the step that swaps them back to `~> 2.0` as part of the 2.0.0 push, so the good pin lands with the release rather than before it. - The Quick Start taught the API 2.1 deletes, silently. `require "capybara_screenshot_diff/minitest"` + `include CapybaraScreenshotDiff::Minitest::Assertions` print nothing: they are eager aliases, so `const_missing` never fires. The Quick Start now starts on canonical `SnapDiff`, and README/CHANGELOG/snapdiff.md say which doors actually warn (config accessors, `include`, `default_options`, `const_missing`) and which cannot. - The CI "Record new baselines" job could not record a new baseline. `check_base_screenshot` runs before `capture_screenshot` and `fail_if_new` is true whenever `ENV["CI"]` is set, so a new screenshot raises before anything is written and the commit step finds nothing. The job now clears `CI` for that step. - `bundle exec rake test` / `rails test` swept out of the three CI workflows, "The Short Version", and the historical upgrade sections: in a Rails app they skip `test/system/` and report `0 runs`. - The delete-the-baselines block in UPGRADING replaced with the commit workflow the README documents. - `docs/drivers.md` told you to delete the `driver:` setting on one screen and to add it on another; same for configuration.md, migration-guide.md. - `rescue SnapDiff::Error` does not catch a failed assertion under the framework integrations -- Minitest converts it to `Minitest::Assertion` and RSpec to `ExpectationNotMetError`. Said so. - "2.0 will not rewrite a baseline you already committed" contradicted the README and reality: a failing run does rewrite the baseline path. Reworded to what is meant (no re-encoding) plus what actually happens. Also, all verified in a scratch app: the `git add test/fixtures/screenshots/` path was never the default (`doc/screenshots` is); the example failure output showed a `max_color_distance` key the vips path never emits; the artifact table listed three of five files; `application_system_test_case.rb` omitted both `require "test_helper"` and `driven_by` (without the latter the same page captures at 2800x1610 instead of 1400x1257); `homepage_test.rb` omitted `require "application_system_test_case"` and raised NameError as printed; and `DEBUG=1` never had anything to do with keeping `.diff.png` files. Docs only. rake test:unit 610/0, standardrb clean. * docs: three residuals from the verifier pass - bug_report template told reporters to run `rake test`, which runs zero system tests in a Rails app -- the same trap this branch exists to remove. - drivers.md said an unknown per-screenshot `driver:` key is "simply inert". It is validated and raises; only the deprecation warning is absent. - drivers.md said `:auto` and `:chunky_png` each warn once per process. `:auto` is silent when ruby-vips resolves; only `:chunky_png` warns.
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
1 participant
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.
2.0 is the transitional release (old and new APIs, with warnings); 2.1 deletes the legacy namespace trees. Several places still said 3.0 — including the runtime migration notice, which ships to users in 2.0.
Changed:
lib/snap_diff/deprecation.rb— the notice now reads "It still works in 2.0 and is REMOVED in 2.1"test/legacy/snap_diff_deprecation_test.rb— the pin wasassert_includes out, "REMOVED in 3.0"; it now asserts the whole clause"still works in 2.0 and is REMOVED in 2.1", so a wrong window fails it too, not just a wrong removal versiontest/unit/core_tree_has_no_legacy_deps_test.rb—DELETED_IN_3_0→DELETED_WITH_LEGACY_TREES(durable phrasing; the version number added nothing)test/unit/deletion_3_0_test.rb→test/unit/legacy_deletion_test.rb,Deletion30Test→LegacyDeletionTestdocs/UPGRADING.mdand ~15 test commentsSupersedes #241, which could not be rebased cleanly: #246 had since landed 2.1 language in
drivers.md/configuration.md, so that branch conflicted with work that overtook it. Its two substantive ideas are carried over here — the whole-clause assertion and the durable naming.standardrb151 files clean ·rake test:unit585/0 ·rake test:canonical482/0/1.🤖 Generated with Claude Code
Summary by Sourcery
Align legacy API migration documentation, runtime notices, and deletion coverage with its removal in 2.1 after the transitional 2.0 release.
Bug Fixes:
Enhancements:
Documentation:
Tests: