Skip to content

Avoid using EXPOSE directive for optional ports#20

Open
yvanzo wants to merge 3 commits intomasterfrom
docker-optional-ports
Open

Avoid using EXPOSE directive for optional ports#20
yvanzo wants to merge 3 commits intomasterfrom
docker-optional-ports

Conversation

@yvanzo
Copy link
Contributor

@yvanzo yvanzo commented May 26, 2022

It follows #18 (review).

yvanzo added 3 commits May 26, 2022 10:50
The idea is to prevent false positives when listing containers:
#18 (comment)

Changes originally submitted by @alastair at:
#18 (comment)
To follow the previous commit, it makes explicit that only mandatory
ports should be exposed through the EXPOSE directive, so as to avoid
false positives when listing containers.
@yvanzo yvanzo requested a review from alastair May 26, 2022 10:04
Copy link
Contributor

@alastair alastair left a comment

Choose a reason for hiding this comment

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

I made some suggestions to the grammar, which I hope maintains the original meaning that you wanted to portray

Comment on lines +7 to +10
with mandatory ports only,
to document which ports the services are necessarily listening to. If the image will be used to run many
containers, some of which don't listen on a port, instead use the `expose` option in
`docker-compose.yml` (see the below section for details) or the `--expose` flag to `docker run`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
with mandatory ports only,
to document which ports the services are necessarily listening to. If the image will be used to run many
containers, some of which don't listen on a port, instead use the `expose` option in
`docker-compose.yml` (see the below section for details) or the `--expose` flag to `docker run`.
only when **all** containers using this image will listen on that port, to document that all services
will be listening on that port. If the image will be used to run many
containers, some of which don't listen on a port, instead use the `expose` option in
`docker-compose.yml` (see the below section for details) or the `--expose` flag to `docker run`
on only the containers which listen on the port.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants