Conversation
There was a problem hiding this comment.
Pull request overview
Initial full-stack project scaffolding for a “Cheat Sheet” app, introducing a React (Vite) frontend, a Django REST backend, and Docker Compose workflow to run both services locally.
Changes:
- Added Vite + React frontend with a simple health-check UI calling
/api/health/. - Added Django + DRF backend with an
/api/health/endpoint and environment-driven settings. - Added Dockerfiles and a
docker-compose.ymlto build/run frontend + backend together.
Reviewed changes
Copilot reviewed 24 out of 28 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/vite.config.js | Vite dev server config (port + API proxy). |
| frontend/src/main.jsx | React entrypoint mounting <App />. |
| frontend/src/index.css | Basic root layout styles. |
| frontend/src/App.jsx | App component that fetches backend health endpoint. |
| frontend/src/App.css | Basic App styling. |
| frontend/package.json | Frontend deps + scripts (dev/build/lint/preview). |
| frontend/package-lock.json | Locked frontend dependency tree for npm. |
| frontend/index.html | Vite HTML entry with root container. |
| frontend/Dockerfile | Containerizes Vite dev server. |
| frontend/.dockerignore | Excludes node_modules/dist/git from frontend build context. |
| docker-compose.yml | Orchestrates backend + frontend containers for local dev. |
| backend/requirements.txt | Django/DRF/CORS/dotenv dependencies. |
| backend/manage.py | Django management entrypoint. |
| backend/cheat_sheet/wsgi.py | WSGI app bootstrap. |
| backend/cheat_sheet/urls.py | Routes /api/ to the api app and enables admin. |
| backend/cheat_sheet/settings.py | Django settings incl. env loading, CORS, DRF defaults. |
| backend/cheat_sheet/asgi.py | ASGI app bootstrap. |
| backend/cheat_sheet/init.py | Package marker. |
| backend/api/views.py | DRF health check view. |
| backend/api/urls.py | API URL routing (health endpoint). |
| backend/api/serializers.py | Placeholder serializer module. |
| backend/api/models.py | Placeholder models module. |
| backend/api/init.py | Package marker. |
| backend/Dockerfile | Containerizes Django dev server. |
| backend/.env.docker | Docker env defaults for Django settings. |
| backend/.dockerignore | Excludes venv/cache/db/env/git from backend build context. |
| README.md | Setup instructions for backend/frontend/docker usage. |
| .gitignore | Repo-wide Python/Node/IDE/OS ignores. |
Files not reviewed (1)
- frontend/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
frontend/package.json
Outdated
| "scripts": { | ||
| "dev": "vite", | ||
| "build": "vite build", | ||
| "lint": "eslint .", |
There was a problem hiding this comment.
npm run lint is defined, but there is no ESLint configuration file in the frontend (e.g., eslint.config.js for ESLint 9, or an .eslintrc*). As-is, eslint . will typically fail with "No ESLint configuration found". Add an ESLint config (and any needed parser/settings for React) or remove/adjust the lint script until configuration is in place.
| "lint": "eslint .", |
frontend/Dockerfile
Outdated
| WORKDIR /app | ||
|
|
||
| COPY package.json package-lock.json* ./ | ||
| RUN npm install |
There was a problem hiding this comment.
For deterministic container builds, prefer npm ci over npm install when a lockfile is present. npm ci is faster and ensures the container matches package-lock.json exactly.
| RUN npm install | |
| RUN npm ci |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Removed lint script from package.json
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 28 changed files in this pull request and generated 10 comments.
Files not reviewed (1)
- frontend/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| frontend: | ||
| build: ./frontend | ||
| ports: | ||
| - "5173:5173" | ||
| volumes: | ||
| - ./frontend:/app | ||
| - /app/node_modules | ||
| depends_on: |
There was a problem hiding this comment.
In Docker Compose, the frontend container cannot reach the backend at http://localhost:8000 (that points to the frontend container itself). Since vite.config.js defaults BACKEND_URL to localhost:8000, the /api proxy will fail under compose unless BACKEND_URL is set to something like http://backend:8000 for the frontend service (or the config detects Docker and uses the service name).
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| ALLOWED_HOSTS = os.getenv("DJANGO_ALLOWED_HOSTS", "localhost,127.0.0.1,0.0.0.0").split(",") | ||
|
|
There was a problem hiding this comment.
ALLOWED_HOSTS is derived via .split(',') without trimming whitespace. If the env var contains spaces (common in .env files), Django will treat entries like ' localhost' as invalid hosts. Consider stripping each entry after splitting (and similarly for other comma-separated lists).
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@Davictory2003 I've opened a new pull request, #7, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@Davictory2003 I've opened a new pull request, #8, to work on those changes. Once the pull request is ready, I'll request review from you. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 28 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- frontend/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "Set it to a securely generated value before running in production." | ||
| ) | ||
|
|
||
| ALLOWED_HOSTS = os.getenv("DJANGO_ALLOWED_HOSTS", "localhost,127.0.0.1,0.0.0.0").split(",") |
There was a problem hiding this comment.
ALLOWED_HOSTS is split on commas without trimming whitespace. If DJANGO_ALLOWED_HOSTS contains spaces (common in .env files), entries like ' localhost' will be treated as invalid hosts. Consider stripping each host (and optionally filtering out empty strings) after splitting.
| ALLOWED_HOSTS = os.getenv("DJANGO_ALLOWED_HOSTS", "localhost,127.0.0.1,0.0.0.0").split(",") | |
| ALLOWED_HOSTS = [ | |
| host.strip() | |
| for host in os.getenv("DJANGO_ALLOWED_HOSTS", "localhost,127.0.0.1,0.0.0.0").split(",") | |
| if host.strip() | |
| ] |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 28 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- frontend/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # DRF | ||
| REST_FRAMEWORK = { | ||
| "DEFAULT_PERMISSION_CLASSES": [ | ||
| "rest_framework.permissions.AllowAny", | ||
| ], | ||
| } |
There was a problem hiding this comment.
The REST_FRAMEWORK configuration uses AllowAny permissions globally, which means all API endpoints are publicly accessible without authentication. While this might be intentional for initial development, it should be documented or reconsidered before adding endpoints that should be protected. Consider adding a comment explaining this is for development only.
No description provided.