-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docker compose overlay file for editable frontend container #1092
Conversation
WalkthroughWalkthroughThe changes streamline the development environment and deployment process for Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 2
Configuration used: .coderabbit.yaml
Files selected for processing (4)
- Makefile (1 hunks)
- spiffworkflow-frontend/dev.Dockerfile (1 hunks)
- spiffworkflow-frontend/dev.docker-compose.yml (1 hunks)
- spiffworkflow-frontend/package.json (1 hunks)
Additional comments: 6
spiffworkflow-frontend/dev.Dockerfile (2)
- 1-1: Using a specific version of the Node.js Docker image (
20.8.1-bookworm-slim
) is good practice for ensuring consistency across development environments. However, consider periodically reviewing and updating the Node.js version to benefit from security patches, performance improvements, and new features.- 9-9: The
CMD
directive specifies the command to run the application usingnpm run docker:start
. This approach is clear and aligns with the PR's objective to streamline the development process. Ensure that thedocker:start
script inpackage.json
is correctly configured to start the application in a development-friendly manner (e.g., enabling hot reloading).spiffworkflow-frontend/dev.docker-compose.yml (1)
- 1-10: The service configuration for
spiffworkflow-frontend
is well-defined, with clear settings for the build context, Dockerfile, environment variables, and volume mount. Using environment variables for configuration (HOST
andPORT
) and a volume mount for live editing are particularly beneficial for a development environment. Ensure that theSPIFFWORKFLOW_FRONTEND_PORT
environment variable is documented and set in an appropriate place (e.g.,.env
file or Makefile) for clarity and ease of use.Makefile (2)
- 4-17: The Makefile targets for managing the development environment (
dev-env
,start-dev
,stop-dev
) are well-structured and use Docker Compose commands effectively. These targets simplify the process of building, starting, and stopping the development environment, aligning with the PR's objectives. Ensure that thedocker-compose.yml
file and theFRONTEND_DEV_OVERLAY
variable are correctly configured and documented for these commands to work as expected.- 19-23: The
fe-lint-fix
andfe-sh
targets provide convenient shortcuts for running lint fixes and accessing a shell within the frontend container, respectively. These additions enhance the development workflow by making common tasks more accessible. Ensure that theFRONTEND_CONTAINER
variable matches the container name defined in the Docker Compose configuration to avoid any discrepancies.spiffworkflow-frontend/package.json (1)
- 69-69: The addition of the
docker:start
script inpackage.json
is a key part of the PR's enhancements to the development environment. This script simplifies the process of starting the application within the Docker container. Ensure that the script's command (ESLINT_NO_DEV_ERRORS=true craco start
) is correctly configured for the intended development workflow, particularly in terms of handling ESLint errors and starting the application.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: .coderabbit.yaml
Files selected for processing (1)
- spiffworkflow-frontend/dev.Dockerfile (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- spiffworkflow-frontend/dev.Dockerfile
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: .coderabbit.yaml
Files selected for processing (1)
- spiffworkflow-frontend/package.json (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- spiffworkflow-frontend/package.json
This provides an overlay file which when used along with the existing
docker-compose.yml
, allows for editing frontend code. Plan to add another overlay for the backend, and one for arena for running./bin/run_pyl
so one can spin up a local dev environment based on the docker containers. Added aMakefile
to manage the docker compose commands since setting/reusing variables across commands is easy.Summary by CodeRabbit