chore: trim .fallowrc (fallow 2.84.0 handles these natively)#180
chore: trim .fallowrc (fallow 2.84.0 handles these natively)#180BartWaardenburg wants to merge 3 commits into
Conversation
Greptile SummaryThis PR trims
Confidence Score: 3/5Not safe to merge as-is: the config and the pinned fallow version are now inconsistent, so CI and local runs will produce different findings than the author observed. The removed entries are all legitimate cleanup for fallow 2.84.0, but the version actually executed by every script in package.json is still 2.62.0. That gap means the three native behaviors being relied on (mock-specifier credits, sqlite builtin, feature-flags default) are absent at runtime, so the trimmed config will re-surface findings rather than stay neutral. The fix is straightforward — bump the version pin — but until that happens the config change and the tooling are out of sync. package.json needs its fallow version pin updated from 2.62.0 to at least 2.84.0 to match the assumptions in .fallowrc.jsonc.
|
| Filename | Overview |
|---|---|
| .fallowrc.jsonc | Removes 13 ignoreDependencies entries, the vitest.config.ts entry, and the feature-flags rule override — all valid for fallow ≥2.84.0, but package.json still pins fallow@2.62.0, making the trimmed config inconsistent with the version actually executed by CI and local scripts. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[npm run fallow] --> B["npx fallow@2.62.0 (pinned in package.json)"]
B --> C{Version check}
C -- "2.62.0 < 2.63.0" --> D["❌ @*/__mocks__ NOT auto-credited\n→ reported as unresolved"]
C -- "2.62.0 < 2.81.0" --> E["❌ node:sqlite NOT a known builtin\n→ reported as unresolved"]
C -- "2.62.0 < 2.84.0" --> F["❌ feature-flags NOT off by default\n→ rule fires unexpectedly"]
C -- "2.62.0 < 2.84.0" --> G["❌ vitest.config.ts NOT auto-detected\n→ missed entry point"]
H["PR goal: fallow@2.84.0"] --> I["✅ All 3 removals safe at ≥2.84.0"]
I -.->|"version not bumped in package.json"| B
Reviews (1): Last reviewed commit: "chore: trim .fallowrc now that fallow 2...." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| { | ||
| "$schema": "https://raw.githubusercontent.com/fallow-rs/fallow/main/schema.json", | ||
| "entry": [ | ||
| "vitest.config.ts", | ||
| "apps/*/app.config.{js,ts}", | ||
| "apps/*/babel.config.{js,cjs,mjs,ts}", |
There was a problem hiding this comment.
All three removals rely on features not present in fallow 2.62.0: @*/__mocks__ virtual-specifier credits landed in 2.63.0, node:sqlite builtin recognition in 2.81.0, and feature-flags defaulting to off in 2.84.0. Because package.json pins every fallow script to npx --yes fallow@2.62.0, anyone running npm run fallow (or the CI lane) will get the old 2.62.0 binary, which lacks those native behaviors and will surface the removed entries as findings again. The version pin in package.json needs to be bumped to at least 2.84.0 alongside this config change, otherwise the two are inconsistent.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughRemoved a vitest entry glob, the ChangesFallow Configuration Cleanup
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
|
Actionable comments posted: 0 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
Good catch from the Greptile review: the scripts were pinned to |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@package.json`:
- Around line 54-62: Update the hardcoded FALLOW_VERSION in
scripts/testing/lib/fallowLocalTooling.test.ts so it matches the package.json
scripts which pin fallow@2.84.0: locate the const FALLOW_VERSION = '2.62.0' and
change it to '2.84.0' (or alternately read the version dynamically from
package.json) so the test's version-matching assertion aligns with the pinned
"fallow@2.84.0" in package.json.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3214edd3-8e42-4820-b6a9-3d192b5c137b
📒 Files selected for processing (1)
package.json
Bump FALLOW_VERSION to 2.84.0 to match the pinned package.json scripts, and drop the obsolete node:sqlite ignoreDependencies assertion (fallow 2.84.0 recognizes node: builtins natively, so the ignore was removed).
|
Actionable comments posted: 0 |
1 participant
fallow 2.84.0 makes part of
.fallowrc.jsoncunnecessary, so this trims it with no change in findings.Removed:
ignoreDependencies: the 12@*/__mocks__entries are now credited automatically as Vitest__mocks__virtual specifiers (since 2.63.0), andnode:sqliteis recognized as a Node builtin (since 2.81.0)vitest.config.tsentryline (auto-detected by the Vitest plugin)rules.feature-flags: "off"override (feature-flagsis off by default)Verified: I ran your repo with the original and the trimmed config and the findings are the same; none of the removed entries re-surface (checked each of the 13 deps plus the vitest config). The other
entryglobs,ignorePatterns, and theduplicates/health/auditblocks are unchanged.Refs #179.
Unrelated heads-up the run surfaced: a lot of
@happier-dev/*imports report as unresolved because their packageexportspoint atdist/that isn't built at analysis time. That's a fallow-side gap I'm tracking (fallow-rs/fallow#757), not something to work around in this config.Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Summary by CodeRabbit