Skip to content

Conversation

@maparent
Copy link
Collaborator

@maparent maparent commented Sep 27, 2025

Make sure that turbo lint does not fail.

https://linear.app/discourse-graphs/issue/ENG-908/make-lint-not-fail-on-database-utils

Summary by CodeRabbit

  • New Features

    • Database migration script now auto-regenerates types after applying migrations.
    • Clearer guidance and exit behavior when migrations require the --include-all flag.
    • Improved error handling, status checks, and messaging, including when no migrations are applied.
  • Chores

    • Updated editor and ESLint configurations (enable flat config, adopt base config, project-aware TypeScript parsing).
    • Added rule to flag constant conditions in code, improving code quality.
    • Minor formatting tweaks in SQL schemas; no functional changes.

@linear
Copy link

linear bot commented Sep 27, 2025

@supabase
Copy link

supabase bot commented Sep 27, 2025

Updates to Preview Branch (eng-908-make-lint-not-fail-on-database-utils) ↗︎

Deployments Status Updated
Database Sun, 28 Sep 2025 02:25:14 UTC
Services Sun, 28 Sep 2025 02:25:14 UTC
APIs Sun, 28 Sep 2025 02:25:14 UTC

Tasks are run on every commit but only new migration files are pushed.
Close and reopen this PR if you want to apply changes from existing seed or migration files.

Tasks Status Updated
Configurations Sun, 28 Sep 2025 02:25:15 UTC
Migrations Sun, 28 Sep 2025 02:25:15 UTC
Seeding Sun, 28 Sep 2025 02:25:15 UTC
Edge Functions Sun, 28 Sep 2025 02:25:18 UTC

View logs for this Workflow Run ↗︎.
Learn more about Supabase for Git ↗︎.

@maparent
Copy link
Collaborator Author

@CodeRabbit review.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 27, 2025

@maparent I'll review the changes in this PR.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 27, 2025

📝 Walkthrough

Walkthrough

Introduces editor LSP settings for ESLint, updates ESLint configs (rule addition, config source change, and new per-package config), adjusts a database migration script to parse stderr and trigger type generation when migrations apply, and reformats Supabase SQL schema files without functional changes.

Changes

Cohort / File(s) Summary of changes
Editor LSP settings
./.zed/settings.json
Adds ESLint LSP settings; enables experimental.useFlatConfig: true.
ESLint base config rule
packages/eslint-config/base.js
Adds rule: "no-constant-condition": ["error", { checkLoops: "allExceptWhileTrue" }].
Package ESLint configs
packages/database/eslint.config.mjs, packages/utils/eslint.config.mjs
Database: switches extend source to @repo/eslint-config/base. Utils: adds config extending base and enables project-based TS parsing (parserOptions.project: true, tsconfigRootDir: ".").
Database migration script
packages/database/scripts/migrate.ts
Captures and parses stderr, adds conditional handling for missing --include-all, exits on non-zero status, detects applied migrations from stderr and runs npm run gentypes when needed; adjusts spawnSync stdio usage.
Supabase schema formatting
packages/database/supabase/schemas/*
account.sql: reformat function signature line only. embedding.sql: trailing newline adjustment. No semantic changes.

Sequence Diagram(s)

sequenceDiagram
  actor Dev as Developer
  participant M as migrate.ts
  participant S as Supabase CLI
  participant N as npm (gentypes)

  Dev->>M: Run migrate script
  M->>S: spawnSync("supabase", ["migrate", ...])
  S-->>M: status, stderr (text)
  alt Non-zero status and stderr suggests "--include-all"
    M->>Dev: Log stderr and retry suggestion
    M->>Dev: Exit with status
  else Continue
    M->>M: Parse stderr for "Applying migration ..."
    alt One or more migrations applied
      M->>N: spawnSync("npm", ["run", "gentypes"], stdio: inherit)
      N-->>M: status
      alt gentypes failed
        M->>Dev: Exit with error
      else
        M->>Dev: Done
      end
    else No migrations applied
      M->>Dev: Log "No migrations applied"
    end
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Pre-merge checks

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title precisely conveys the pull request’s core objective—to modify linting so it no longer fails for the database and utils packages—which directly aligns with the ESLint configuration updates and rule additions in those areas.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@maparent maparent requested a review from mdroidian September 27, 2025 17:38
Copy link
Contributor

@mdroidian mdroidian left a comment

Choose a reason for hiding this comment

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

Could you clarify what problem you are trying to solve?

I assumed it was turbo lint or turbo lint -F @repo/database but both of those are still failing for me.

Image

@maparent
Copy link
Collaborator Author

It was a global lint, and worked for me. Out now, I'll check again later tonight.

Copy link
Collaborator Author

ok I put an eslint command that seemed to work in my editor and not in eslint. Corrected.

@maparent maparent requested a review from mdroidian September 28, 2025 01:11
* Apply sqlruff lints
* One valid lint on migrate.ts
* Tweak lint rules to allow `while (true)`
* Wrong lint base in @repo/database
@maparent maparent force-pushed the eng-908-make-lint-not-fail-on-database-utils branch from 3aaf87a to 183aed1 Compare September 28, 2025 01:13
Copy link
Contributor

@mdroidian mdroidian left a comment

Choose a reason for hiding this comment

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

turbo lint still fails for me, same error.

Image

Also, on the place that it is erroring, I see this lint:

Image

Copy link
Collaborator Author

ok I tried something else that made a warning go away on my side, it may solve your error?

Copy link
Contributor

@mdroidian mdroidian left a comment

Choose a reason for hiding this comment

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

👍

@maparent maparent merged commit 8464cf1 into main Sep 28, 2025
5 checks passed
@github-project-automation github-project-automation bot moved this to Done in General Sep 28, 2025
@maparent maparent deleted the eng-908-make-lint-not-fail-on-database-utils branch September 28, 2025 04:13
trangdoan982 pushed a commit that referenced this pull request Oct 3, 2025
* * Missing eslint.config in utils
* Apply sqlruff lints
* One valid lint on migrate.ts
* Tweak lint rules to allow `while (true)`
* Wrong lint base in @repo/database
* remove eslint9-dependent setting.
* declare eslint as module
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants