docs: 2.0.0 release readiness - #251
Conversation
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.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
Warning Review limit reachedNext included review available in 37 minutes. View limit detailsLimit 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. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (14)
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 |
Reviewer's GuideDocs and packaging updates to make 2.0.0 ready for final release: clarify transitional nature and 2.1 removals, document dual-gem naming and deprecations, fix release workflow links, tighten gemspec allow-list and metadata, add tests for packaged contents, and update contributor/release runbooks and driver docs (including silent Flow diagram for the dual-gem release pipelineflowchart LR
Maintainer["Maintainer dispatches release workflow"] --> Verify["Verify version and run unit tests"]
Verify --> Tag["Create or reuse v<version> tag"]
Tag --> PublishPrimary["Publish capybara-screenshot-diff"]
PublishPrimary --> PublishMirror["Build and publish snap_diff-capybara mirror"]
PublishMirror --> Release["Create GitHub Release with tag-pinned docs links"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
…ot 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.
|
One more false doc claim found while verifying the CHANGELOG, fixed in 92c80a7.
Related |
…e tag push Branch protection rules do not govern tag pushes; tag protection rules (or rulesets) do. The runbook prerequisite now names the right control.
…tting 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.
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.
Second pass: customer-persona findings, verifiedEvery item below was re-verified here before acting. Two of the four reported defects turned out to be artifacts of the published beta3, not of master — that distinction changes the verdict, so the evidence is spelled out. Verdict on the deprecation-window item — not a 2.0.0 blocker, but beta3 must never be recommended againThe v1-upgrader persona is completely right about what they measured, and the fix is already on master.
I then ran the persona's exact Stage 1 — Gemfile bumped, zero code changed, v1 require + v1
So It does mean something sharper: anyone sitting on a 2.0.0 prerelease has been told, by silence, that they are migrated. The CHANGELOG now carries a callout saying exactly that. Every beta pin was already removed from the docs in the first commit; this is the reason it mattered. Gem name: yes, shipping 2.0.0 final is a prerequisiteConfirmed by fetching and unpacking it — The decision stands and gets firmer: Separately: unpinned Baselines — the 2018 bug, now a first-class sectionConfirmed from source, not just from the reports. New "Accepting an intentional change" section covering the mechanism, the commit that actually accepts the change, and the counter-intuitive part both customers hit: staging is not enough, and you cannot get a green local run until you commit. The FAQ answer now says the same thing instead of the opposite. I deliberately did not document I also stopped short of asserting post-run disk mechanics I could not verify without a Capybara session, so the section routes the reader through CHANGELOG additions — verified, not relayed
Not appliedThe unsilenceable every-run Gates
|
|
Superseded by #257, which contains all five commits from this branch rebased onto master, plus the Quick Start |
1 participant
Audit of everything a 2.0.0 final would ship, plus the fixes that did not require touching
lib/. Do not merge without reading the blocker list — two items need a decision that is not mine to make.Version not bumped, nothing tagged, nothing published.
Blockers, ranked
1.
driver:is deleted in 2.1 and 2.0 never warns about it.SnapDiff.config.driver = :vips, the legacyCapybara::Screenshot::Diff.driver = :vips, and the per-screenshotdriver:override are completely silent on 2.0 and raiseNoMethodErroron 2.1 (#249's own body says so).driver: :autois equally silent on any machine that has libvips — the:autowarning only fires when:autoactually falls back to ChunkyPNG. That contradicteddocs/drivers.md, which promised everything 2.1 removes "warns once per process naming 2.1".Evidence — a
SnapDiff.configureblock settingdriver = :vips, then a comparison, produced zero output:2.0's whole justification is that publication mitigates the semver departure, so a silent removal is the one thing that should not be in it. What this PR did: documented it as silent in
docs/drivers.mdanddocs/UPGRADING.md, and stopped the setup examples teachingdriver: :vips. What is still open (needslib/, and a decision): either add aRemoval.warn_onceonConfig#driver=mirroring the existingshift_distance_limit=override, or — better, and lazier for users — have 2.1 keepdriver=as a deprecated no-op instead of deleting it. I lean to the second: warning on the recommended configuration is noise, and a no-op setter costs one line in #249 versus aNoMethodErrorin everyone'stest_helper.rb.2.
Bundler.requirecrashes whenminitestis not in the bundle.lib/capybara-screenshot-diff.rb(and the newlib/snap_diff-capybara.rb) unconditionally requirecapybara_screenshot_diff/minitest, which doesrequire "minitest". Any project without minitest in its Gemfile — an RSpec-only or non-Rails suite — dies at boot:Pre-existing: 1.15.1 fails identically, so it is not a 2.0 regression and I did not fix it (
lib/is another lane's). But 2.0 extends it to the new gem name, and a fresh adopter following the README hits it.lib/owner's call. Rails apps are unaffected (activesupport depends on minitest).3. The README's quick start produces a setup that cannot compare.
gem 'ruby-vips' # Optionalwas not true: with neitherruby-vipsnorchunky_pngthe gem raisesRuntimeError: Wrong adapter nil. Available adapters: []. Fixed in this PR (README now says the gem ships no backend and names the error).4. Every GitHub Release so far links to a 404.
release.ymlpointed the release body atblob/main/CHANGELOG.md; the default branch ismaster. Fixed — links resolve at the tag now, which cannot drift.5.
RELEASE_PREP.mdwas a v1.15.1 checklist. It described neither the dual-name publish, nor trusted publishing, nor prereleases, nor what to do when a run fails after tagging. Rewritten as a runbook.CONTRIBUTING.mdwas worse: it pointed atlib/capybara/screenshot/diff/version.rb(which no longer holds the version, so the workflow's verify step would reject your release) and offeredrake releaseas an alternative — which publishes onlycapybara-screenshot-diffand silently skips the mirror.6. The README still teaches the API 2.1 deletes. Quick start, config examples and the compare snippet are all
Capybara::Screenshot::Diff/CapybaraScreenshotDiff, so a brand-new 2.0 adopter writes code that warns immediately and breaks at 2.1. Deliberately not fixed here — it is a README rewrite, not a version sweep, and it overlaps the fresh-adopter lane. Flagging it as the largest remaining 2.0 embarrassment.The gem-name decision
capybara-screenshot-diffis the name we tell people to install.snap_diff-capybarastays published as an identical mirror so the forward-looking name is reserved and resolvable, but nothing recommends it.Reasoning: every existing user, badge, and third-party blog post points at the old name, so recommending the new one to newcomers buys nothing and creates the exact configuration that trips
SnapDiff::DualInstallErrorthe first time two Gemfiles merge. The mirror can become primary at 2.1, when the rename actually lands. Stated once in the README banner, with the dual-install consequence; every install snippet in the repo already agreed, so this was a framing fix rather than a sweep.Packaged gem inventory
gem build→ 93 files, 776K.lib/snap_diff/, legacycapybara/+capybara_screenshot_diff/trees, bothBundler.requireentry files, the HTML report templatedocs/README.md,LICENSE.txt,CHANGELOG.mdcapybara (>= 2, < 4)— correct and minimal; every image backend is an optional requirebindir/executableslines removed — the allow-list never matchedexe/)Removed from the package:
docs/docker-testing.md(documentsbin/dtest, which is not packaged). Already correctly excluded:docs/RELEASE_PREP.md, tests, CI,Rakefile,gems.rb, the gemspec. For contrast, the 1.15.1 gem shippedRakefile,gems.rb, the gemspec andCODE_OF_CONDUCT.md, and had no README — rubygems.org showed nothing.Added
test/unit/gemspec_packaging_test.rbto pin all of the above. Verified it actually fails: unpackaginglib/snap_diff-capybara.rbturns the entry-point assertion red.What a 1.15.1 user actually sees
Real installs on ruby 4.0.6, scratch projects outside the repo, master pulled in via
path:.Before (1.15.1), legacy
test_helper.rb, legacy config block, legacy constants — clean output.After (this master), same file, nothing changed but the Gemfile line:
Nothing broke. Three warnings, all actionable, all pointing at 2.1. The only surprise worth calling out is the last two lines: constants print under their
SnapDiff::names, so a CI job that string-matches the old class name in output needs updating. That is in the CHANGELOG's "five things that can actually break".A canonical-names setup (
require "snap_diff/integrations/minitest"+SnapDiff.configure) is completely silent — zero warnings.Two more, both green:
Bundler.requireundergem "capybara-screenshot-diff"and undergem "snap_diff-capybara", each from its own built.gemwith the mirror gemspec generated exactly the wayrelease.ymlgenerates it. And the dual-install guard, with both gems genuinely installed into oneGEM_HOME:One incidental improvement worth a CHANGELOG line: 2.0 dropped the undeclared
activesupportruntime requirement. 1.15.1 requiresactive_support/core_ext/module/attribute_accessorswithout declaring the dependency, so a non-Rails install fails to load. Reproduced, then confirmed master needs nothing butcapybara.Open issue sweep
Nothing blocks. #217 (fiber/thread-safety hardening) says so explicitly in its own non-goals; its "document the config contract — configure before parallelism starts" item is a cheap doc win for 2.0 if someone wants it. #166 is the v2 umbrella and should close with the release. #249 is the 2.1 draft and still needs the two human items in
.ai/HANDOFF.md. Two docs sent custom-driver maintainers to #166 while a third sent them to the issue tracker; now consistent.Still unresolved
3.0comments remain inlib/(snap_diff.rb,config.rb,drivers.rb,dsl.rb,legacy_shims.rb,capybara/screenshot/diff/config_legacy.rb). Comments only, no behavior. Left alone becauselib/is another lane's; theRakefileanddocs/architecture.mdcopies are fixed here.CHANGELOG.md's v2.0.0 heading saysunreleased. The release step sets the date —docs/RELEASE_PREP.mdnow lists it.defined?… unchanged" claim. Left as history; the v2.0.0 entry states the real behavior, which I re-verified (const_defined?isfalsefor lazily shimmed names).Gates
rake test:unit590 runs / 0 failures ·rake test:canonical487 / 0 ·rake test618 / 0 (1 pre-existing skip, unchanged from master) ·standardrbclean (161 files) ·yamllintpasses. All on ruby 4.0.6. The five new runs aregemspec_packaging_test.rb.Summary by Sourcery
Prepare the repository and release documentation for the 2.0 transitional release while validating the packaged gem and documenting remaining migration blockers.
New Features:
Bug Fixes:
Enhancements:
Deployment:
Documentation:
Tests:
Chores: