Skip to content

Conversation

@lpmi-13
Copy link

@lpmi-13 lpmi-13 commented Dec 17, 2025

Description

This commit updates the docker configuration for the project in the following ways:

  • moves the ARG lower down in the 2 Dockerfiles to take advantage of layer caching if the image is rebuilt with a different ARG value (the layers above it don't need to be rebuilt).
  • removes unnecessary mkdir statements, since WORKDIR runs mkdir under the hood (https://docs.docker.com/reference/dockerfile/#workdir)
  • binds the compose stack to the loopback interface instead of all interfaces, which means if running on an unsecured subnet (eg, coffee shop wifi) no other hosts will be able to send traffic to the container.

🤝 Attestations

  • My proposed changes don't update any code and are only updates to the container configuration

@vidplace7 vidplace7 self-assigned this Dec 17, 2025
# Forward the container’s port 4403 to the host
ports:
- 4403:4403
- 127.0.0.1:4403:4403
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you actually break this out as a variable (Add in .env.example) and default to 127.0.0.1?

Beyond that nitpick this PR looks good ❤️ thanks for the submission!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, can do 🫡

I was actually wondering, given the nature of this project, if there are some times you'd like it to run on one host and be reachable from another (or maybe there's some other reason I'm missing about why you'd want it bound to 0.0.0.0/0...CI maybe?).

@vidplace7 vidplace7 added the cleanup Code cleanup or refactor label Dec 17, 2025
@lpmi-13
Copy link
Author

lpmi-13 commented Dec 22, 2025

I also added the .env file to the .gitignore, but if it's intentional that it's not in there, I can easily take it out and add a comment about why we don't want it.

@lpmi-13 lpmi-13 requested a review from vidplace7 December 22, 2025 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cleanup Code cleanup or refactor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants