Skip to content

Conversation

@nygrenh
Copy link
Member

@nygrenh nygrenh commented Jan 3, 2026

Using pnpm is unnecessary, and we don't want to accidentally start downloading new versions of pnpm or Node.js (which the project is otherwise configured to automatically do)

Summary by CodeRabbit

  • Chores
    • Optimized production deployment configuration across all services
    • Streamlined startup process by using direct Node.js execution in production containers
    • Consolidated environment variable configuration to Docker layer
    • Removed redundant startup scripts from package configuration

✏️ Tip: You can customize this high-level summary in your review settings.

Using pnpm is unnecessary, and we don't want to accidentally start downloading new versions of pnpm or Node.js (which the project is otherwise configured to automatically do)
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 3, 2026

📝 Walkthrough

Walkthrough

Consolidates Node.js startup configuration from package.json scripts into Docker production images. Removes start scripts from six service package.json files and updates corresponding Dockerfiles to directly invoke node server.js with explicit NODE_ENV and PORT environment variables.

Changes

Cohort / File(s) Summary
Dockerfile Production Updates
services/cms/Dockerfile.production.slim.dockerfile, services/course-material/Dockerfile.production.slim.dockerfile, services/example-exercise/Dockerfile.production.slim.dockerfile, services/main-frontend/Dockerfile.production.slim.dockerfile, services/quizzes/Dockerfile.production.slim.dockerfile, services/tmc/Dockerfile.production.slim.dockerfile
Replaced CMD [ "pnpm", "run", "start" ] with explicit ENV NODE_ENV=production, ENV PORT=<service-port>, and CMD ["node", "server.js"] across all production Dockerfiles
Package Configuration Scripts
services/cms/package.json, services/course-material/package.json, services/example-exercise/package.json, services/main-frontend/package.json, services/quizzes/package.json, services/tmc/package.json
Removed start scripts from scripts section (e.g., "start": "NODE_ENV=production PORT=3000 node server.js")

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • Update deps 2025-10 #1570: Modifies the same startup/entrypoint configuration by changing service package.json start scripts and Dockerfile production CMDs (pnpm vs node invocation).

Suggested labels

chore

Poem

🐰 From npm scripts to Docker's care,
We shuttle startup everywhere!
No more pnpm's gentle call,
Node runs server, that is all.
Ports and envs, neat and small,
Configuration stands so tall!

Pre-merge checks

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: removing pnpm from the startup commands in slim Docker images and replacing with direct Node.js execution.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4f0dff6 and ca906fd.

📒 Files selected for processing (12)
  • services/cms/Dockerfile.production.slim.dockerfile
  • services/cms/package.json
  • services/course-material/Dockerfile.production.slim.dockerfile
  • services/course-material/package.json
  • services/example-exercise/Dockerfile.production.slim.dockerfile
  • services/example-exercise/package.json
  • services/main-frontend/Dockerfile.production.slim.dockerfile
  • services/main-frontend/package.json
  • services/quizzes/Dockerfile.production.slim.dockerfile
  • services/quizzes/package.json
  • services/tmc/Dockerfile.production.slim.dockerfile
  • services/tmc/package.json
💤 Files with no reviewable changes (5)
  • services/quizzes/package.json
  • services/main-frontend/package.json
  • services/tmc/package.json
  • services/cms/package.json
  • services/course-material/package.json
🧰 Additional context used
📓 Path-based instructions (1)
**/*

⚙️ CodeRabbit configuration file

**/*: Avoid banner-style section divider comments ("comment banners"), such as long separator bars, repeated dashes, or loud numbered headers used to split a file into sections (e.g. // ----------------------------------------------------------------------, // 1. INPUT STATE, /* ======= */). Prefer clearer naming, smaller focused modules/files, and normal short comments; do not suggest adding new comment banners, and when they appear in new or modified code, suggest removing them and improving structure instead.
Prefer comment quality over comment quantity. Do not add comments that simply restate what the code already says, narrate control flow, or explain obvious syntax — these may help readers unfamiliar with programming, but they hinder developers by adding noise and slowing down code reading. Comments should be concise, factual, and purposeful, and should communicate information that is not immediately obvious from the code itself (e.g., rationale and intent, constraints or invariants, edge cases, surprising behavior, important assumptions, performance or correctness considerations, or things future maintainers must keep in mind).
Require consistent, minimal function documentation. While some simple or self-explanatory functions or components may not strictly need comments, it is generally preferable to include a short, plain-language description of what the function is for, to clarify intent and complement (not repeat) a descriptive function name. Comments should add meaning rather than paraphrasing the name or implementation. Public interfaces should have clearer and more comprehensive documentation, but many private or internal functions also benefit from a brief comment explaining their role, assumptions, or relevant context.
Protect useful existing comments. Do not remove comments that capture non-obvious rationale, warnings, historical context, or design decisions unless they are clearly outdated or made irrelevant by the change. If a modification ...

Files:

  • services/cms/Dockerfile.production.slim.dockerfile
  • services/example-exercise/Dockerfile.production.slim.dockerfile
  • services/tmc/Dockerfile.production.slim.dockerfile
  • services/main-frontend/Dockerfile.production.slim.dockerfile
  • services/example-exercise/package.json
  • services/course-material/Dockerfile.production.slim.dockerfile
  • services/quizzes/Dockerfile.production.slim.dockerfile
⏰ 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: code-style
  • GitHub Check: build-and-system-tests
  • GitHub Check: headless_lms
🔇 Additional comments (7)
services/quizzes/Dockerfile.production.slim.dockerfile (1)

33-36: LGTM! Clean migration to direct Node.js startup.

The explicit environment variables and direct node server.js invocation eliminate the pnpm runtime dependency as intended. The PORT value correctly matches the EXPOSE directive.

services/course-material/Dockerfile.production.slim.dockerfile (1)

34-37: LGTM! Consistent with the migration pattern.

The changes correctly eliminate pnpm at runtime and explicitly configure the production environment. Port 3003 aligns with the EXPOSE directive.

services/main-frontend/Dockerfile.production.slim.dockerfile (1)

37-40: LGTM! Proper production configuration.

The migration to direct Node.js startup is correctly implemented with explicit environment variables. The configuration aligns with the exposed port.

services/example-exercise/Dockerfile.production.slim.dockerfile (1)

33-36: LGTM! Consistent migration to direct Node.js invocation.

The changes properly configure the production environment and eliminate the pnpm runtime dependency. Port 3002 correctly matches the EXPOSE directive.

services/cms/Dockerfile.production.slim.dockerfile (1)

35-38: LGTM! Completes the consistent migration pattern.

This final service correctly adopts the direct Node.js startup approach with explicit production configuration. The PORT value aligns with the EXPOSE directive. All five services now follow a uniform, maintainable pattern that achieves the PR objective of eliminating pnpm at runtime.

services/tmc/Dockerfile.production.slim.dockerfile (1)

38-41: Dockerfile changes correctly implement Next.js standalone mode.

The CMD ["node", "server.js"] is appropriate because output: "standalone" in next.config.js generates a standalone server.js during the Next.js build, which the Dockerfile correctly copies from .next/standalone to /app. The environment variables NODE_ENV=production and PORT=3005 are properly set. The package.json correctly has no start script, successfully eliminating pnpm in the runtime container.

services/example-exercise/package.json (1)

5-9: No issues found. The Dockerfile.production.slim.dockerfile already contains the required configuration: ENV NODE_ENV=production, ENV PORT=3002, and CMD ["node", "server.js"]. The removal of the start script from package.json is properly supported by these settings.


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.

@socket-security
Copy link

@coderabbitai coderabbitai bot added the chore label Jan 3, 2026
@nygrenh nygrenh merged commit 3777020 into master Jan 3, 2026
14 of 15 checks passed
@nygrenh nygrenh deleted the inline-start-commands branch January 3, 2026 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants