Conversation
Summary of ChangesHello @drobilc, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors the local development environment by transitioning from a custom script-based orchestration to a fully containerized Docker Compose setup. This change aims to standardize how development services are initiated and managed, providing a more consistent and isolated environment for all developers. The update simplifies the setup process, improves dependency management, and enhances the overall developer experience by leveraging Docker's capabilities for service coordination and live code reloading. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a great step towards containerizing the development environment, which will improve consistency and onboarding. The migration from a custom script to Docker Compose is well-executed. I've identified a few areas for improvement:
- The
docker-compose.dev.ymlfile has significant repetition that can be reduced using YAML anchors for better maintainability. - The
devscript inpackage.jsondoesn't forward arguments, which breaks the documented--buildfunctionality. - The
README.mdcontains an incorrect command for rebuilding containers.
My review includes suggestions to address these points.
| "scripts": { | ||
| "dev": "infisical run -- bun -b concurrently --names \"DOCKER,APP\" -c \"blue,magenta\" \"bun run db:start\" \"bun run db:wait && bun run dev:app\"", | ||
| "dev:app": "concurrently --names \"VITE,STUDIO\" -c \"cyan,yellow\" \"vite dev\" \"bun run db:studio\"", | ||
| "dev": "infisical export --format=dotenv > .env.docker && docker compose -f docker-compose.dev.yml up", |
There was a problem hiding this comment.
The dev script doesn't forward arguments to docker compose up. This means bun dev --build (as mentioned in README.md) won't work as expected because the --build flag isn't passed to Docker Compose. To fix this, you can wrap the command in sh -c to correctly handle and forward any arguments.
| "dev": "infisical export --format=dotenv > .env.docker && docker compose -f docker-compose.dev.yml up", | |
| "dev": "sh -c 'infisical export --format=dotenv > .env.docker && docker compose -f docker-compose.dev.yml up \"$@\"' --", |
| # One-time task to run migrations | ||
| migrator: | ||
| build: | ||
| context: . | ||
| dockerfile: Dockerfile.dev | ||
| command: bun drizzle-kit migrate | ||
| depends_on: | ||
| postgres: | ||
| condition: service_healthy | ||
| env_file: .env.docker | ||
| environment: | ||
| - DATABASE_URL=postgresql://${POSTGRES_USER:-postgres}:${POSTGRES_PASSWORD:-postgres}@postgres:${POSTGRES_PORT:-5432}/${POSTGRES_DB:-postgres} | ||
|
|
||
| vite: | ||
| build: | ||
| context: . | ||
| dockerfile: Dockerfile.dev | ||
| env_file: .env.docker | ||
| environment: | ||
| - WATCHPACK_POLLING=true | ||
| - DATABASE_URL=postgresql://${POSTGRES_USER:-postgres}:${POSTGRES_PASSWORD:-postgres}@postgres:${POSTGRES_PORT:-5432}/${POSTGRES_DB:-postgres} | ||
| command: bun run dev:app --host=0.0.0.0 | ||
| ports: | ||
| - "5173:5173" | ||
| volumes: | ||
| - .:/app # Edits on host are reflected in container | ||
| - /app/node_modules | ||
| depends_on: | ||
| postgres: | ||
| condition: service_healthy | ||
| migrator: | ||
| condition: service_completed_successfully | ||
|
|
||
| studio: | ||
| build: | ||
| context: . | ||
| dockerfile: Dockerfile.dev | ||
| env_file: .env.docker | ||
| environment: | ||
| - DATABASE_URL=postgresql://${POSTGRES_USER:-postgres}:${POSTGRES_PASSWORD:-postgres}@postgres:${POSTGRES_PORT:-5432}/${POSTGRES_DB:-postgres} | ||
| command: bun run db:studio --port 4983 --host 0.0.0.0 | ||
| ports: | ||
| - "4983:4983" | ||
| volumes: | ||
| - .:/app # Edits on host are reflected in container | ||
| - /app/node_modules | ||
| depends_on: | ||
| postgres: | ||
| condition: service_healthy | ||
| migrator: | ||
| condition: service_completed_successfully |
There was a problem hiding this comment.
The definitions for migrator, vite, and studio services share a lot of common configuration, such as build, env_file, and environment. This repetition can make the file harder to maintain. Consider using YAML anchors and aliases to reduce duplication. For example, you could define a common build block and reuse it:
x-common-build: &common-build
build:
context: .
dockerfile: Dockerfile.dev
services:
migrator:
<<: *common-build
# ... rest of migrator config
vite:
<<: *common-build
# ... rest of vite configYou can apply a similar pattern for other repeated sections like environment or volumes.
There was a problem hiding this comment.
Literally no one can read this. I think we can leave as is.
There was a problem hiding this comment.
what do you mean, arent we all yaml fans and experts
| **Note on dependencies**: Since `node_modules` are cached, you must rebuild the containers if you add or change packages. You can do this by running: | ||
|
|
||
| ```bash | ||
| bun dev --build |
There was a problem hiding this comment.
The command bun dev --build is not the correct way to pass arguments to a bun script; this passes the --build flag to bun itself, not the script. To pass arguments to the script, you should use bun dev -- --build. Please update the documentation to reflect the correct command usage.
| bun dev --build | |
| bun dev -- --build |
| migrator: | ||
| build: | ||
| context: . | ||
| dockerfile: Dockerfile.dev | ||
| command: bun drizzle-kit migrate | ||
| depends_on: | ||
| postgres: | ||
| condition: service_healthy | ||
| env_file: .env.docker | ||
| environment: | ||
| - DATABASE_URL=postgresql://${POSTGRES_USER:-postgres}:${POSTGRES_PASSWORD:-postgres}@postgres:${POSTGRES_PORT:-5432}/${POSTGRES_DB:-postgres} |
There was a problem hiding this comment.
a ideja je, da maš prazno bazo k je prpravlena, če pa hočš staging bazo jo pa separately itak popolnoma povozš (torej ni nekih konfliktov?)
| # One-time task to run migrations | ||
| migrator: | ||
| build: | ||
| context: . | ||
| dockerfile: Dockerfile.dev | ||
| command: bun drizzle-kit migrate | ||
| depends_on: | ||
| postgres: | ||
| condition: service_healthy | ||
| env_file: .env.docker | ||
| environment: | ||
| - DATABASE_URL=postgresql://${POSTGRES_USER:-postgres}:${POSTGRES_PASSWORD:-postgres}@postgres:${POSTGRES_PORT:-5432}/${POSTGRES_DB:-postgres} | ||
|
|
||
| vite: | ||
| build: | ||
| context: . | ||
| dockerfile: Dockerfile.dev | ||
| env_file: .env.docker | ||
| environment: | ||
| - WATCHPACK_POLLING=true | ||
| - DATABASE_URL=postgresql://${POSTGRES_USER:-postgres}:${POSTGRES_PASSWORD:-postgres}@postgres:${POSTGRES_PORT:-5432}/${POSTGRES_DB:-postgres} | ||
| command: bun run dev:app --host=0.0.0.0 | ||
| ports: | ||
| - "5173:5173" | ||
| volumes: | ||
| - .:/app # Edits on host are reflected in container | ||
| - /app/node_modules | ||
| depends_on: | ||
| postgres: | ||
| condition: service_healthy | ||
| migrator: | ||
| condition: service_completed_successfully | ||
|
|
||
| studio: | ||
| build: | ||
| context: . | ||
| dockerfile: Dockerfile.dev | ||
| env_file: .env.docker | ||
| environment: | ||
| - DATABASE_URL=postgresql://${POSTGRES_USER:-postgres}:${POSTGRES_PASSWORD:-postgres}@postgres:${POSTGRES_PORT:-5432}/${POSTGRES_DB:-postgres} | ||
| command: bun run db:studio --port 4983 --host 0.0.0.0 | ||
| ports: | ||
| - "4983:4983" | ||
| volumes: | ||
| - .:/app # Edits on host are reflected in container | ||
| - /app/node_modules | ||
| depends_on: | ||
| postgres: | ||
| condition: service_healthy | ||
| migrator: | ||
| condition: service_completed_successfully |
There was a problem hiding this comment.
what do you mean, arent we all yaml fans and experts
| "scripts": { | ||
| "dev": "infisical run -- bun -b concurrently --names \"DOCKER,APP\" -c \"blue,magenta\" \"bun run db:start\" \"bun run db:wait && bun run dev:app\"", | ||
| "dev:app": "concurrently --names \"VITE,STUDIO\" -c \"cyan,yellow\" \"vite dev\" \"bun run db:studio\"", | ||
| "dev": "infisical export --format=dotenv > .env.docker && docker compose -f docker-compose.dev.yml up", |
| "scripts": { | ||
| "dev": "infisical run -- bun -b concurrently --names \"DOCKER,APP\" -c \"blue,magenta\" \"bun run db:start\" \"bun run db:wait && bun run dev:app\"", | ||
| "dev:app": "concurrently --names \"VITE,STUDIO\" -c \"cyan,yellow\" \"vite dev\" \"bun run db:studio\"", | ||
| "dev": "infisical export --format=dotenv > .env.docker && docker compose -f docker-compose.dev.yml up", |
There was a problem hiding this comment.
just a normal infisical run does not work? damn
2 participants
This pull request migrates the development workflow to use a fully containerized Docker-based setup, streamlining how services are started and managed. The changes remove the custom Node/Bun orchestrator script in favor of Docker Compose, introduce a new development Dockerfile, and update documentation and configuration files accordingly.
Development workflow overhaul:
scripts/dev.ts) with a Docker Compose-based approach for managing all dev services. ([scripts/dev.tsL1-L108](https://github.com/zerodays/full-stack-starter/pull/4/files#diff-653680a4ce21b1015ba542020323435170fd8e5803dd2edd75c2ae70c9cfb075L1-L108))Dockerfile.devfor building the development environment using the Bun runtime. ([Dockerfile.devR1-R8](https://github.com/zerodays/full-stack-starter/pull/4/files#diff-86930c95a19b82f7e64a962a0053d44e855824813019b3698eae4917a90cdcacR1-R8)).dockerignoreto exclude unnecessary files from Docker build context, improving build efficiency. ([.dockerignoreR1-R15](https://github.com/zerodays/full-stack-starter/pull/4/files#diff-2f754321d62f08ba8392b9b168b83e24ea2852bb5d815d63e767f6c3d23c6ac5R1-R15))docker-compose.dev.ymlto define separate services for the app (vite), database migrations (migrator), and Drizzle Studio (studio), all depending on a healthy PostgreSQL instance. Services now share code via volumes for live reload. ([docker-compose.dev.ymlR19-R70](https://github.com/zerodays/full-stack-starter/pull/4/files#diff-9542f82d64bbeebd91f6236324bfe199e9657e2cb1fd9779d5d6dcdcf9cd4de1R19-R70))Configuration and dependency updates:
package.jsonscripts to launch the dev environment via Docker Compose and removed theconcurrentlydependency. ([[1]](https://github.com/zerodays/full-stack-starter/pull/4/files#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519L7-R8),[[2]](https://github.com/zerodays/full-stack-starter/pull/4/files#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519L39))README.mdto reflect the new Docker-based workflow and explain how to rebuild containers when dependencies change. ([README.mdL48-R60](https://github.com/zerodays/full-stack-starter/pull/4/files#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5L48-R60))