From 5bb42bb303d59e136306a95a1b692d94e30c5b9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20V=C3=A1squez?= Date: Mon, 4 May 2026 14:50:40 -0600 Subject: [PATCH] Accept optional kind: field in detection patterns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extends bin/validate-patterns to recognize a `kind:` field on every pattern entry, with enum validation against four allowed values: `breaking`, `deprecation`, `migration`, `optional`. The validator rejects unknown values (typo guard) but does not require the field yet — existing pattern files keep validating clean. Documents the four values and assignment rubric in CLAUDE.md, alongside the existing priority rubric. priority and kind are orthogonal: a HIGH deprecation (silently wrong) and a HIGH breaking (won't boot) are both "fix first" but for different reasons. First step of the issue #53 rollout. Subsequent classification PRs will add `kind:` per pattern, version-by-version. After all 12 pattern files are classified, the validator will be tightened to require kind. Refs #53, closes #54 --- CLAUDE.md | 24 ++++++++++++++++++++++-- bin/validate-patterns | 5 +++++ rails-upgrade/CHANGELOG.md | 3 +++ 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 1e14066..9f4998c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -7,7 +7,7 @@ This file captures project-specific conventions Claude should follow when workin - `bin/validate-patterns` validates every detection pattern YAML file under `rails-upgrade/detection-scripts/patterns/`. Run it before committing any change to a pattern file. Pure-stdlib Ruby, no Bundler or Gemfile required. - `bin/validate-patterns` validates every file - `bin/validate-patterns path/to/file.yml` validates one or more specific files - - Checks: YAML parses, required top-level keys present, the seven required pattern keys present on each entry, and every `pattern` / `exclude` regex compiles + - Checks: YAML parses, required top-level keys present, the seven required pattern keys present on each entry, every `pattern` / `exclude` regex compiles, and any `kind:` value (optional during the issue #53 rollout) is one of the allowed enum values - Exits 0 on success, 1 on any failure with a per-file error report ## Version guides (`rails-upgrade/version-guides/*.md`) @@ -22,7 +22,7 @@ This file captures project-specific conventions Claude should follow when workin - File naming: `rails-{VERSION}-patterns.yml` where `{VERSION}` is the major+minor without a dot (e.g., `rails-42-patterns.yml` for Rails 4.2). - Organize patterns under `high_priority`, `medium_priority`, and `low_priority`. -- Each pattern needs: `name`, `pattern` (regex), `exclude` (regex, empty string if none), `search_paths`, `explanation`, `fix`, `variable_name`. +- Each pattern needs: `name`, `pattern` (regex), `exclude` (regex, empty string if none), `search_paths`, `explanation`, `fix`, `variable_name`. New entries should also set `kind:` (see "Assigning kind" below); the field is optional during the issue #53 rollout and becomes required once every pattern file has been classified. - Include a `dependencies` section for any bridge/compatibility gems mentioned in the guide. - **Required before committing any change to a detection pattern file:** run `bin/validate-patterns` (or `bin/validate-patterns path/to/file.yml` for the file you touched). Do not commit a pattern change without a clean run; broken YAML or schema drift in this directory breaks the skill at runtime. See the `## Repository tooling` section above for what the script checks. @@ -65,3 +65,23 @@ If most apps will ignore it without consequence, it is LOW. 3. **Would the app be unaffected unless the user opts into a new feature or runs in a specific environment?** → LOW. Priority is about **urgency during an upgrade**, not editorial weight. + +## Assigning `kind:` (`breaking` / `deprecation` / `migration` / `optional`) + +`kind:` describes **what the change is**; `priority` describes **how urgent it is**. The two are orthogonal. A HIGH `deprecation` (silently wrong, like `DIRTY_TRACKING_AFTER_SAVE`) and a HIGH `breaking` (won't boot) are both "fix first" but for different reasons. + +The four values: + +- **`breaking`** — Raises, removed, or prevents the app from booting / bundling / running its test suite. The user cannot complete the upgrade without addressing it. Example: `update_attributes` removed in 6.1, `redirect_to :back` removed in 5.1. +- **`deprecation`** — Works at this hop but emits a deprecation warning. Removal is scheduled for a later Rails version. Example: dynamic `:controller` route segments in 5.2, string `if:` conditions on callbacks. +- **`migration`** — Works today, no warning, but a recommended migration target. Adopting it now avoids rework on the next hop or an entirely new approach. Example: `Rails.application.secrets` → `credentials.yml.enc` in 5.2. +- **`optional`** — Opt-in feature or improvement. The user can ignore it without consequence. Example: `bootsnap`, `webpacker` in 5.1, `propshaft` adoption ahead of 8.0. + +How to decide: + +1. **Will the upgrade fail (boot, bundle, tests) without this fix?** → `breaking`. +2. **Does Rails emit a deprecation warning when this code runs at the target version?** → `deprecation`. +3. **Is this a recommended path forward (e.g. `secrets.yml` → `credentials.yml.enc`) that does not yet warn?** → `migration`. +4. **Is this purely opt-in / cosmetic / a new feature?** → `optional`. + +If `kind` and `priority` seem to conflict, trust both. They answer different questions. diff --git a/bin/validate-patterns b/bin/validate-patterns index 843688a..a7a2c8f 100755 --- a/bin/validate-patterns +++ b/bin/validate-patterns @@ -15,6 +15,7 @@ PATTERNS_DIR = File.expand_path("../rails-upgrade/detection-scripts/patterns", _ TOP_LEVEL_KEYS = %w[version description breaking_changes].freeze PATTERN_KEYS = %w[name pattern exclude search_paths explanation fix variable_name].freeze PRIORITY_KEYS = %w[high_priority medium_priority low_priority].freeze +KIND_VALUES = %w[breaking deprecation migration optional].freeze def validate(path) errors = [] @@ -50,6 +51,10 @@ def validate(path) errors << "#{label}: missing key: #{k}" unless entry.key?(k) end + if entry.key?("kind") && !KIND_VALUES.include?(entry["kind"]) + errors << "#{label}: invalid kind: #{entry["kind"].inspect} (allowed: #{KIND_VALUES.join(", ")})" + end + %w[pattern exclude].each do |key| val = entry[key] next if key == "exclude" && (val.nil? || val.to_s.empty?) diff --git a/rails-upgrade/CHANGELOG.md b/rails-upgrade/CHANGELOG.md index a2ab3f3..e57abcc 100644 --- a/rails-upgrade/CHANGELOG.md +++ b/rails-upgrade/CHANGELOG.md @@ -1,5 +1,8 @@ # Changelog +## Unreleased +- `bin/validate-patterns` now accepts an optional `kind:` field on every pattern entry. Allowed values: `breaking`, `deprecation`, `migration`, `optional`. The validator rejects unknown values to guard against typos. The field is optional during the issue #53 rollout and becomes required once every pattern file has been classified. Documented the rubric in `CLAUDE.md` under "Assigning `kind:`". + ## v3.3.0, 28 April 2026 - Added `upgrade-cleanup` companion plugin for finishing a Rails upgrade campaign. Removes dual-boot scaffolding, drops `NextRails.next?` / `NextRails.current?` branches, retires stale monkey-patches and version-conditional code, and aligns CI matrix / Dockerfile / Ruby pin to the new version baseline. Lives as a sibling plugin in this repo (`upgrade-cleanup/.claude-plugin/plugin.json` + `upgrade-cleanup/upgrade-cleanup/SKILL.md` + `upgrade-cleanup/upgrade-cleanup/workflows/upgrade-cleanup-workflow.md`). `load_defaults` alignment and deprecation triage stay with the rails-upgrade skill, cleanup deliberately does not duplicate them. Based on FastRuby.io's "Finishing an Upgrade" methodology. (Closes #1) - Cleanup is **user-triggered, never automatic**. After each Rails upgrade ships, the rails-upgrade skill now mentions the cleanup option but does not run it. Between hops in a multi-hop campaign the user usually wants to keep dual-boot in place and roll straight into the next hop. Cleanup ends the campaign.