feat: allow overriding docker compose published port#146
feat: allow overriding docker compose published port#146f440 wants to merge 1 commit intomatt1398:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the docker-compose.yml file to allow the host port for the claude-devtools service to be configured via the PORT environment variable, defaulting to 3456. The reviewer suggests documenting this new configuration option in the README.md to ensure it is discoverable for users.
| build: . | ||
| ports: | ||
| - "3456:3456" | ||
| - "${PORT:-3456}:3456" |
There was a problem hiding this comment.
While this change correctly allows overriding the host port, it introduces a new way to configure the service that isn't documented. To make this feature discoverable for users, please consider updating the README.md file.
A good place would be in the Docker / Standalone Deployment section, adding a note and an example of how to use the PORT environment variable to change the published port, like so:
# Run on a different port
PORT=4000 docker compose up
📝 WalkthroughWalkthroughThe Changes
Suggested labels
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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docker-compose.yml (1)
4-8: Quick-start comment is now misleading for non-defaultPORT.Line 7 always points to
http://localhost:3456, but that’s only true whenPORTis not overridden. Consider documenting default + override behavior here (and mirror it inREADME.mdLine 82).✏️ Suggested doc tweak
# Quick start: # docker compose up # -# Then open http://localhost:3456 in your browser. +# Then open http://localhost:${PORT:-3456} in your browser. +# (Default is 3456; override with `PORT=<host_port>`.)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker-compose.yml` around lines 4 - 8, Update the quick-start comment in docker-compose.yml to avoid hard-coding "http://localhost:3456": state the default is 3456 (via PORT) and show how to override it (e.g., export PORT=xxxx or use environment in docker compose) and provide a generic URL example like "http://localhost:<PORT>". Also update the corresponding note in README.md (the paragraph referenced around line 82) to mirror this default + override behavior so both places stay consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docker-compose.yml`:
- Around line 4-8: Update the quick-start comment in docker-compose.yml to avoid
hard-coding "http://localhost:3456": state the default is 3456 (via PORT) and
show how to override it (e.g., export PORT=xxxx or use environment in docker
compose) and provide a generic URL example like "http://localhost:<PORT>". Also
update the corresponding note in README.md (the paragraph referenced around line
82) to mirror this default + override behavior so both places stay consistent.
Allow the Docker Compose host-side published port to be overridden via
PORTwhile keeping the container-side app port fixed at3456.This helps tools such as Portless, which launch a process with a dynamically assigned host port and then proxy to that
port. With this change, a command like
portless run docker compose upcan work without changing the app's internal listen port.Summary by CodeRabbit