docs: the legacy API is removed in 2.1, not 3.0 - #241
Closed
pftg wants to merge 1 commit into
Closed
Conversation
The repo told users the v1 Capybara::Screenshot* / CapybaraScreenshotDiff* API is "REMOVED in 3.0" -- in the runtime migration notice that ships in 2.0, in UPGRADING.md, and in ~50 code comments. Per ADR-008's final amendment there is no 3.0: 2.0.0 final ships WITH the full compatibility layer, and 2.1 deletes the legacy trees, legacy_shims.rb, deprecation.rb, test/legacy/, the chunky_png driver and shift_distance_limit. Shipping the old text would publish a promise we intend to break. - Migration notice now reads "still works in 2.0 and is REMOVED in 2.1"; the pinning assertion in snap_diff_deprecation_test.rb matches the whole clause instead of just the version, and was mutation-checked. - UPGRADING.md states the contract plainly up front: legacy names work in 2.0 with warnings, all of it removed in 2.1. - architecture.md reframes legacy_shims.rb as the removal layer rather than a permanent fixture, and names the gates that keep the removal a git rm. - drivers.md / configuration.md now say chunky_png and shift_distance_limit are deprecated in 2.0 and removed in 2.1 (docs only; the runtime warnings are separate work). - deletion_3_0_test.rb -> legacy_deletion_test.rb, DELETED_IN_3_0 -> DELETED_WITH_LEGACY_TREES. Version-free phrasing elsewhere so the comments stop carrying a release number that adds nothing. - CHANGELOG gains an Unreleased corrective note; the published beta sections are left as history.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
Warning Review limit reached
Next review available in: 5 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 (34)
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 documentation, migration messaging, and test gates with the decision that the legacy v1 API, ChunkyPNG driver, and Sequence diagram for the legacy API migration noticesequenceDiagram
participant App
participant LegacyAPI
participant Deprecation
App->>LegacyAPI: access legacy API
LegacyAPI->>Deprecation: warn_once
Deprecation-->>App: migration notice: works in 2.0, removed in 2.1
LegacyAPI-->>App: compatibility behavior
Note over Deprecation: Notice shown once per process
Flow diagram for the legacy API removal timelineflowchart LR
V20["2.0.0 final"] --> W["Legacy compatibility layer works and warns\nMigration window"]
W --> V21["2.1"]
V21 --> R["Remove v1 namespaces, shims,\ndeprecation machinery, and legacy tests"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Screenshot diffs detected
|
Collaborator
Author
|
Superseded by #247. This branch could not be rebased cleanly — #246 landed 2.1 language in docs/drivers.md and docs/configuration.md in the meantime, so the sweep conflicted with work that overtook it. Both substantive ideas are carried over in #247: the notice test now asserts the whole clause rather than the version alone, and DELETED_IN_3_0 became DELETED_WITH_LEGACY_TREES. |
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.
The repo currently tells users the v1 API is "REMOVED in 3.0" — in the runtime
migration notice that ships inside 2.0, in
docs/UPGRADING.md, and in ~50 codecomments. Per ADR-008's final amendment there is no 3.0:
per-constant deprecation warnings, one-per-process migration notice.
legacy_shims.rb,deprecation.rb,test/legacy/, and the chunky_png driver +shift_distance_limit.If the current text ships in 2.0 we publish a promise we intend to break, and #240
cannot fix it because it removes the file that carries the text.
The user-facing string
before
after
A test does pin it:
test/legacy/snap_diff_deprecation_test.rb:189. It wasasserting only
"REMOVED in 3.0"; it now matches the whole clause"still works in 2.0 and is REMOVED in 2.1", so a wrong window fails it too andnot just a wrong removal version. Mutation-checked: setting the notice to
REMOVED in 9.9makes it fail with the expected diff.The per-constant warning text carries no version at all
(
`X` is deprecated (constant); use `Y` instead.) and is left alone — themigration notice always precedes the first constant warning in a process, so the
version is stated exactly once rather than on every line.
Per-file breakdown
lib/snap_diff/deprecation.rbtest/legacy/snap_diff_deprecation_test.rbdocs/UPGRADING.mdNameErrorsentence,CONFIG_MAPPING; new plain statement of the contract at the top of "Deprecation Warnings"docs/architecture.mdlegacy_shims.rbreframed as the removal layer, not a permanent fixture, naming the three gates; dropped a stale "3.0-readiness pass"docs/drivers.mddocs/configuration.mdshift_distance_limitdeprecation calloutCHANGELOG.md[Unreleased]corrective note (see below)Rakefiletest/unit/deletion_3_0_test.rb→test/unit/legacy_deletion_test.rbDeletion30Test→LegacyDeletionTesttest/unit/core_tree_has_no_legacy_deps_test.rbDELETED_IN_3_0→DELETED_WITH_LEGACY_TREEStest/unit/canonical_suite_has_no_legacy_refs_test.rbtest/legacy/*.rb3.0→2.1lib/snap_diff{.rb,/config.rb,/drivers.rb,/dsl.rb,/legacy_shims.rb},lib/capybara/.../config_legacy.rbtest/unit/{support_load_probe,snap_diff,snap_diff_config,config_default_timing,drivers,errors,image_compare}_test.rb,test/support/{dsl_stub,driver_coverage}.rbLeft alone deliberately
test/unit/backtrace_filter_test.rb:35—"/gems/rack-3.0.0/lib/rack.rb", a Rack version in a fixture backtrace.test/unit/drivers/vips_driver_test.rb:52—"[11.0,3.0,49.0,21.0]", region coordinates.docs/UPGRADING.md:313-344,CHANGELOG.md:226—v1.13.0, matched only because3.0is a substring.CHANGELOG.md:103("removing them in 3.0 is a deletion") and:96("v3's scroll-preservation work") — publishedv2.0.0.beta3history, not rewritten. The[Unreleased]note covers them.The prior scan's counts did not hold up:
docs/configuration.md,docs/framework-setup.mdandREADME.mdcontain zero3.0matches (claimed 31/19/13), anddocs/UPGRADING.mdhad 7, not 62 — 4 of which arev1.13.0.CHANGELOG judgment call
The CHANGELOG does promise 3.0, at line 103 in the published
v2.0.0.beta3section ("…so removing them in 3.0 is a deletion, not a refactor").
beta3istagged, so I did not rewrite it. There was no
[Unreleased]section, so I addedone carrying two corrective bullets: the legacy API moves to 2.1 (with "read every
earlier mention of 3.0 as 2.1"), and chunky_png +
shift_distance_limitgo at 2.1too. No
version.rbbump.Overlap with #240 (draft)
#240 also renames some of this. Where a version number added nothing I used durable
phrasing so the branches conflict as little as possible:
deletion_3_0_test.rb→legacy_deletion_test.rb/Deletion30Test→LegacyDeletionTestDELETED_IN_3_0→DELETED_WITH_LEGACY_TREES2.1is used only where the sentence is the removal contract (the notice, thedocs,
legacy_shims.rb's andconfig_legacy.rb's file headers, the Rakefile gate).Everything under
test/legacy/uses2.1— those files are deleted by #240 anyway.Verification
rake test:unit→ 572 runs, 0 failures (baseline 572/0)rake test→ 600 runs, 0 failures, 1 skip (baseline 600/0/1)rake test:canonical→ 469 runs, 0 failures, 1 skipstandardrb→ 158 files, no offensesREMOVED in 9.9→ red)Do not merge — for reconciliation with #240 first.
Summary by Sourcery
Correct the project’s release and migration messaging to establish 2.0 as the legacy compatibility window and 2.1 as the removal release.
Enhancements:
shift_distance_limit, including their removal in 2.1.Build:
Documentation:
shift_distance_limit, including recommended VIPS-based replacements.Tests:
Chores: