Skip to content

Conversation

@TorstenDittmann
Copy link
Contributor

@TorstenDittmann TorstenDittmann commented Oct 27, 2025

Summary by CodeRabbit

Release Notes

  • Chores
    • Updated deployment configuration paths for production and staging environments
    • Upgraded ESLint, Prettier, and related development tooling to latest versions for improved code quality and consistency checks

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 27, 2025

Walkthrough

This pull request contains three primary changes: migration of Docker Compose file paths in GitHub Actions workflows (moving from docker/ to deploy/docker/ directories), refactoring of ESLint configuration to use a defineConfig wrapper instead of ts.config, reorganization of ESLint configuration structure with consolidation of rule disables and removal of top-level ignores, and version bumps across ESLint-related packages, Prettier ecosystem, and the globals package.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50–70 minutes

  • eslint.config.js: Multiple structural changes including switching default export pattern from ts.config(...) to defineConfig(...), reorganization of imports, removal of ignores declarations, and consolidation of rule disables require careful verification that ESLint functionality remains intact and that the new wrapper properly initializes configuration.
  • package.json: Version bumps across multiple dependencies (ESLint ^9.36.0, eslint-config-prettier ^10.1.8, eslint-plugin-svelte ^3.12.4, prettier ^3.6.2, globals ^16.4.0) need verification of compatibility and confirmation that no breaking changes are introduced by these upgrades.
  • .github/workflows/production.yml and .github/workflows/staging.yml: Path changes for STACK_FILE variables require confirmation that the new deployment file paths (deploy/docker/production.yml and deploy/docker/stage.yml) exist and are correctly configured for deployment.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check ❓ Inconclusive The title "fix: tidy up files" is vague and generic, using non-descriptive language that fails to convey meaningful information about the changeset. While the PR does include multiple changes that could loosely be characterized as cleanup or reorganization—such as ESLint configuration refactoring with defineConfig replacement, dependency version bumps, and workflow path adjustments—the title doesn't specify any of these concrete changes. A developer scanning commit history would have no clear understanding of what was actually modified or the rationale behind these changes. The term "tidy up" is too broad and could apply to almost any maintenance-related PR. Consider revising the title to be more specific and descriptive of the actual changes. A better title might reference the primary changes, such as "refactor: update ESLint config and bump dependencies" or "chore: consolidate ESLint configuration and update workflow paths". This would help reviewers and future maintainers quickly understand the scope and purpose of the changeset.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-linter

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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
package.json (1)

31-31: Remove duplicate posthog-js from devDependencies

A duplicate posthog-js exists with different versions: ^1.210.2 in dependencies (line 31) and ^1.204.0 in devDependencies (line 89). Remove it from devDependencies and keep the higher version in dependencies.

🧹 Nitpick comments (8)
.github/workflows/staging.yml (3)

21-27: Rename step to reflect GHCR, not DockerHub

You’re logging into ghcr.io. Rename for clarity.

-            - name: Login to DockerHub
+            - name: Login to GHCR

35-51: Pass tokens as BuildKit secrets, not build-args

GITHUB_TOKEN, SENTRY_AUTH_TOKEN, and Appwrite keys should be secrets to avoid leaking into image layers.

                   build-args: |
                       "PUBLIC_APPWRITE_ENDPOINT=${{ vars.PUBLIC_APPWRITE_ENDPOINT }}"
                       "PUBLIC_APPWRITE_DASHBOARD=${{ vars.PUBLIC_APPWRITE_DASHBOARD }}"
                       "PUBLIC_APPWRITE_PROJECT_ID=${{ vars.PUBLIC_APPWRITE_PROJECT_ID }}"
                       "PUBLIC_APPWRITE_DB_MAIN_ID=${{ vars.PUBLIC_APPWRITE_DB_MAIN_ID }}"
                       "PUBLIC_APPWRITE_COL_THREADS_ID=${{ vars.PUBLIC_APPWRITE_COL_THREADS_ID }}"
                       "PUBLIC_APPWRITE_COL_MESSAGES_ID=${{ vars.PUBLIC_APPWRITE_COL_MESSAGES_ID }}"
                       "PUBLIC_APPWRITE_FN_TLDR_ID=${{ vars.PUBLIC_APPWRITE_FN_TLDR_ID }}"
                       "PUBLIC_APPWRITE_PROJECT_INIT_ID=${{ vars.PUBLIC_APPWRITE_PROJECT_INIT_ID }}"
                       "PUBLIC_GROWTH_ENDPOINT=${{ vars.PUBLIC_GROWTH_ENDPOINT }}"
                       "PUBLIC_POSTHOG_API_KEY=${{ vars.PUBLIC_POSTHOG_API_KEY }}"
-                      "APPWRITE_DB_INIT_ID=${{ secrets.APPWRITE_DB_INIT_ID }}"
-                      "APPWRITE_COL_INIT_ID=${{ secrets.APPWRITE_COL_INIT_ID }}"
-                      "APPWRITE_API_KEY_INIT=${{ secrets.APPWRITE_API_KEY_INIT }}"
-                      "GITHUB_TOKEN=${{ secrets.GITHUB_TOKEN }}"
-                      "SENTRY_AUTH_TOKEN=${{ secrets.SENTRY_AUTH_TOKEN }}"
+                  secrets: |
+                      "APPWRITE_DB_INIT_ID=${{ secrets.APPWRITE_DB_INIT_ID }}"
+                      "APPWRITE_COL_INIT_ID=${{ secrets.APPWRITE_COL_INIT_ID }}"
+                      "APPWRITE_API_KEY_INIT=${{ secrets.APPWRITE_API_KEY_INIT }}"
+                      "GITHUB_TOKEN=${{ secrets.GITHUB_TOKEN }}"
+                      "SENTRY_AUTH_TOKEN=${{ secrets.SENTRY_AUTH_TOKEN }}"

Follow-up: update Dockerfile to use RUN --mount=type=secret,id=… to consume these.


1-7: Set explicit permissions for GHCR push

Harden defaults and ensure packages: write is available.

 name: Staging deployment
 
 on:
     workflow_dispatch:
     push:
         branches:
             - main
+permissions:
+  contents: read
+  packages: write
.github/workflows/production.yml (3)

21-27: Rename to GHCR login

Same as staging.

-            - name: Login to DockerHub
+            - name: Login to GHCR

35-51: Use BuildKit secrets for tokens/keys

Move sensitive args to secrets. Keep SENTRY_RELEASE as build-arg.

                   build-args: |
                       "PUBLIC_APPWRITE_ENDPOINT=${{ vars.PUBLIC_APPWRITE_ENDPOINT }}"
                       "PUBLIC_APPWRITE_DASHBOARD=${{ vars.PUBLIC_APPWRITE_DASHBOARD }}"
                       "PUBLIC_APPWRITE_PROJECT_ID=${{ vars.PUBLIC_APPWRITE_PROJECT_ID }}"
                       "PUBLIC_APPWRITE_DB_MAIN_ID=${{ vars.PUBLIC_APPWRITE_DB_MAIN_ID }}"
                       "PUBLIC_APPWRITE_COL_THREADS_ID=${{ vars.PUBLIC_APPWRITE_COL_THREADS_ID }}"
                       "PUBLIC_APPWRITE_COL_MESSAGES_ID=${{ vars.PUBLIC_APPWRITE_COL_MESSAGES_ID }}"
                       "PUBLIC_APPWRITE_FN_TLDR_ID=${{ vars.PUBLIC_APPWRITE_FN_TLDR_ID }}"
                       "PUBLIC_APPWRITE_PROJECT_INIT_ID=${{ vars.PUBLIC_APPWRITE_PROJECT_INIT_ID }}"
                       "PUBLIC_GROWTH_ENDPOINT=${{ vars.PUBLIC_GROWTH_ENDPOINT }}"
                       "PUBLIC_POSTHOG_API_KEY=${{ vars.PUBLIC_POSTHOG_API_KEY }}"
-                      "APPWRITE_DB_INIT_ID=${{ secrets.APPWRITE_DB_INIT_ID }}"
-                      "APPWRITE_COL_INIT_ID=${{ secrets.APPWRITE_COL_INIT_ID }}"
-                      "APPWRITE_API_KEY_INIT=${{ secrets.APPWRITE_API_KEY_INIT }}"
-                      "GITHUB_TOKEN=${{ secrets.GITHUB_TOKEN }}"
-                      "SENTRY_AUTH_TOKEN=${{ secrets.SENTRY_AUTH_TOKEN }}"
                       "SENTRY_RELEASE=${{ github.event.release.tag_name }}"
+                  secrets: |
+                      "APPWRITE_DB_INIT_ID=${{ secrets.APPWRITE_DB_INIT_ID }}"
+                      "APPWRITE_COL_INIT_ID=${{ secrets.APPWRITE_COL_INIT_ID }}"
+                      "APPWRITE_API_KEY_INIT=${{ secrets.APPWRITE_API_KEY_INIT }}"
+                      "GITHUB_TOKEN=${{ secrets.GITHUB_TOKEN }}"
+                      "SENTRY_AUTH_TOKEN=${{ secrets.SENTRY_AUTH_TOKEN }}"

1-7: Add explicit permissions

Needed for GHCR push; improves security posture.

 name: Production deployment
 
 on:
     release:
         types: [published]
     workflow_dispatch:
+permissions:
+  contents: read
+  packages: write
eslint.config.js (1)

25-28: Scope no-undef rule to TS/Svelte files only

The codebase has 14 plain JS files alongside 182 TS and 409 Svelte files. The current global no-undef: 'off' disables checking in those JS files unnecessarily. The typescript-eslint guidance applies to TypeScript sources specifically; JS files should retain the check to catch real issues. The configuration already supports scoped rules (as shown by the Svelte block), so moving this rule into a files-scoped config is both feasible and aligned with the codebase structure.

         rules: {
-            // typescript-eslint strongly recommend that you do not use the no-undef lint rule on TypeScript projects.
-            // see: https://typescript-eslint.io/troubleshooting/faqs/eslint/#i-get-errors-from-the-no-undef-rule-about-global-variables-not-being-defined-even-though-there-are-no-typescript-errors
-            'no-undef': 'off'
         }
     },
+    {
+        files: ['**/*.ts', '**/*.tsx', '**/*.svelte', '**/*.svelte.ts', '**/*.svelte.js'],
+        rules: {
+            // typescript-eslint strongly recommend that you do not use the no-undef lint rule on TypeScript projects.
+            // see: https://typescript-eslint.io/troubleshooting/faqs/eslint/#i-get-errors-from-the-no-undef-rule-about-global-variables-not-being-defined-even-though-there-are-no-typescript-errors
+            'no-undef': 'off'
+        }
+    },
     {
         files: ['**/*.svelte', '**/*.svelte.ts', '**/*.svelte.js'],
package.json (1)

71-76: Add Node engine declaration to ensure CI/dev parity with ESLint 9, SvelteKit, and Vite 6

ESLint 9 requires Node >= 18.18.0, and the codebase uses @sveltejs/kit ^2.42.2 (Node 18+ minimum) and Vite ^6.2.4. Currently, no engines field exists in package.json. Adding "engines": { "node": ">=18.18" } declares the minimum Node version needed, preventing subtle failures when developers or CI environments use incompatible Node versions.

   "packageManager": "[email protected]",
+  "engines": {
+    "node": ">=18.18"
+  },
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d7ca606 and 6dc5d85.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (4)
  • .github/workflows/production.yml (1 hunks)
  • .github/workflows/staging.yml (1 hunks)
  • eslint.config.js (1 hunks)
  • package.json (3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: tests
  • GitHub Check: build
  • GitHub Check: format
🔇 Additional comments (2)
.github/workflows/production.yml (1)

10-10: Remove unused STACK_FILE environment variable

The STACK_FILE variable defined at line 10 is never referenced anywhere in the workflow. The deployment uses Helm (see helm upgrade step), not Docker Compose, making this variable obsolete. Removing it reduces configuration drift.

.github/workflows/staging.yml (1)

11-11: Remove unused STACK_FILE environment variable

STACK_FILE is defined at line 11 but never referenced in any workflow step. The deployment uses Helm, not Docker Stack files, so this variable is dead code. Remove it or wire it into a step if needed.

@coderabbitai coderabbitai bot mentioned this pull request Oct 27, 2025
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.

3 participants