Skip to content

chore(repo): fold the effect lint into the repo lint (CLI-2899) - #7422

Open
7ttp wants to merge 6 commits into
developfrom
7ttp/cli-2899-fold-the-effect-lint-into-the-repo-lint
Open

7ttp wants to merge 6 commits into
developfrom
7ttp/cli-2899-fold-the-effect-lint-into-the-repo-lint

Conversation

@7ttp

@7ttp 7ttp commented Oct 30, 2026 •

Copy link
Copy Markdown
Member

TL;DR

folds the effect lint into pnpm lint:check, so one .oxlintrc.json drives editors, agents and CI

whats introduced?

  • .oxlintrc.json extends the @effect/tsgo recommended preset, and .oxlintrc.effect.json and the lint:effect:* scripts go away
  • .github/scripts, the npm launcher shim, compute stack templates and the generated API contracts move to ignorePatterns, so they leave the repo lint entirely
  • lint:fix applies suggestions, and fix:all runs knip, lint and format in order so fixes never race

ref

@7ttp 7ttp self-assigned this Oct 30, 2026
@7ttp
7ttp requested review from a team and supabase-oss as code owners October 30, 2026 17:54

@github-actions github-actions Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Superseded by a newer AI review

🤖 AI Review

Both independent reviews completed. Confirmed Claude's maintenance nit: Effect exclusions depend on a manually synchronized rule list. No additional issues found. Lint execution was unavailable because dependencies are not installed.

Findings

Severity Location Category Sources Claim
⚪ NIT .oxlintrc.json:585 maintainability claude The Effect exclusions disable rules individually, so future preset additions can apply to previously excluded files unless this list is updated. Resulting diagnostics can fail CI.

Stats

Claude findings: 1 · Codex findings: 0 · Confirmed: 1 · Refuted: 0 · Uncertain: 0


Models: claude-opus-5-5 + gpt-6.1-sol · Trigger: auto · Workflow run

This review runs once per PR. A maintainer can request another with a /ai-review comment.

Comment thread .oxlintrc.json Resolved
@github-actions

Copy link
Copy Markdown
Contributor

CLI benchmark: no changes found

Commit a94226a against develop at e713e69 · 10 runs each · report only, never blocks merging

Command Before After Change
supabase --version 183 ms 187 ms no real change
supabase --help 195 ms 185 ms no real change (-5%, within normal variation)
supabase init 176 ms 178 ms no real change
supabase migration new bench 180 ms 179 ms no real change
supabase gen types --lang typescript --project-id replayfixtureprojref 215 ms 208 ms no real change
supabase projects list 182 ms 175 ms no real change
supabase functions list --project-ref replayfixtureprojref 208 ms 223 ms no real change (+7%, within normal variation)

API calls: unchanged for all 7 commands.

Binary size: 134.0 MB, unchanged.

Details: timing ranges, API calls per command, slowest phases

Timing ranges

Command Before range After range Change Verdict
supabase --version 174-238 ms 171-248 ms +4 ms (+2%) within noise
supabase --help 168-215 ms 172-207 ms -10 ms (-5%) within noise
supabase init 163-197 ms 157-199 ms +1 ms (+1%) within noise
supabase migration new bench 157-245 ms 156-228 ms 0 ms (0%) within noise
supabase gen types --lang typescript --project-id replayfixtureprojref 189-262 ms 195-229 ms -7 ms (-3%) within noise
supabase projects list 170-206 ms 168-240 ms -7 ms (-4%) within noise
supabase functions list --project-ref replayfixtureprojref 187-276 ms 173-304 ms +15 ms (+7%) within noise

API calls per command

Command Before After Endpoint changes
supabase --version 0 0
supabase --help 0 0
supabase init 0 0
supabase migration new bench 0 0
supabase gen types --lang typescript --project-id replayfixtureprojref 2 2
supabase projects list 1 1
supabase functions list --project-ref replayfixtureprojref 2 2

Slowest phases

No command moved beyond noise, so there are no phase deltas.

Comment /bench to run it again on the latest commit.

Workflow run

@7ttp

7ttp commented Oct 30, 2026

Copy link
Copy Markdown
Member Author

/ai-review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 AI Review

Both independent reviews completed. Confirmed Claude’s minor lint-coverage finding: the consolidated configuration excludes CI scripts from all oxlint checks. Codex reported no findings. Verification was static because node_modules is absent.

Findings

Severity Location Category Sources Claim
🟡 MINOR .oxlintrc.json:17 lint-coverage claude Adding .github/scripts to the global ignorePatterns removes all oxlint coverage for CI automation, rather than excluding only Effect-specific rules.

Stats

Claude findings: 1 · Codex findings: 0 · Confirmed: 1 · Refuted: 0 · Uncertain: 0


Models: claude-opus-5-5 + gpt-6.1-sol · Trigger: manual · Workflow run

This review runs once per PR. A maintainer can request another with a /ai-review comment.

Comment thread .oxlintrc.json Resolved

@kanadgupta kanadgupta left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mind adding comments for all the ignores/exclusions?

@7ttp
7ttp requested a review from kanadgupta October 30, 2026 22:35

@kanadgupta kanadgupta left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one more piece of feedback, apologies

Comment thread package.json
"check:all": "pnpm exec turbo run types:check lint:check fmt:check knip:check lint:effect:check lint:actions:check",
"fix:all": "pnpm exec turbo run lint:fix fmt:fix knip:fix && pnpm run lint:effect:fix",
"check:all": "pnpm exec turbo run types:check lint:check fmt:check knip:check lint:actions:check",
"fix:all": "pnpm exec turbo run fmt:fix --continue=always",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the context on this change (i.e., making it so pnpm fix:all only lists fmt:fix as its sole task and we use the dependsOn tree in turbo.json)?

i think the previous approach (where the tasks were listed out in this turbo run command) makes more sense to me, for a few reasons:

  • these tasks not actually dependent on one another (but correct me if i'm wrong here)
  • if i as a user wanted to fix oxfmt issues, i'd expect that running pnpm exec turbo run fmt:fix would only run oxfmt and not the other fixing commands
  • it's antipattern compared to check:all

if the reasoning for this change is that we can't run these three commands in parallel, we should use --concurrency 1 instead

suggestion if these can run in parallel:

Suggested change
"fix:all": "pnpm exec turbo run fmt:fix --continue=always",
"fix:all": "pnpm exec turbo run lint:fix fmt:fix knip:fix",

suggestion if these cannot run in parallel:

Suggested change
"fix:all": "pnpm exec turbo run fmt:fix --continue=always",
"fix:all": "pnpm exec turbo run lint:fix fmt:fix knip:fix --concurrency 1 --continue=always",

Comment thread turbo.json
Comment on lines +18 to +19
"cache": false,
"dependsOn": ["knip:fix"]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

per my feedback above

Suggested change
"cache": false,
"dependsOn": ["knip:fix"]
"cache": false

Comment thread turbo.json
Comment on lines +28 to +29
"cache": false,
"dependsOn": ["lint:fix"]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

per my feedback above

Suggested change
"cache": false,
"dependsOn": ["lint:fix"]
"cache": false

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants