Skip to content

refactor(cmd/compose/run): remove redundant len check#11060

Merged
glours merged 1 commit intodocker:mainfrom
Juneezee:refactor/redundant-len-check
Oct 23, 2023
Merged

refactor(cmd/compose/run): remove redundant len check#11060
glours merged 1 commit intodocker:mainfrom
Juneezee:refactor/redundant-len-check

Conversation

@Juneezee
Copy link
Contributor

@Juneezee Juneezee commented Oct 2, 2023

What I did

  1. Remove redundant len(v) > 0 checks in docker compose run command.

    From the Go specification (https://go.dev/ref/spec#For_range):

    "1. For a nil slice, the number of iterations is 0."

    len returns 0 if the slice is nil (https://pkg.go.dev/builtin#len). Therefore, checking len(v) > 0 before a loop is unnecessary.

    Also it is unnecessary to initialise target.Ports = []types.ServicePortConfig{}. append works on nil slices (https://go.dev/tour/moretypes/15).

  2. Update existing test case for docker compose run --publish and add a new test case for docker compose run --service-ports to verify the documented behaviour.

    The second difference is that the `docker compose run` command does not create any of the ports specified in the
    service configuration. This prevents port collisions with already-open ports. If you do want the service’s ports
    to be created and mapped to the host, specify the `--service-ports`
    ```console
    $ docker compose run --service-ports web python manage.py shell
    ```
    Alternatively, manual port mapping can be specified with the `--publish` or `-p` options, just as when using docker run:
    ```console
    $ docker compose run --publish 8080:80 -p 2022:22 -p 127.0.0.1:2021:21 web python manage.py shell
    ```

Related issue

(not mandatory) A picture of a cute animal, if possible in relation to what you did

image

@Juneezee
Copy link
Contributor Author

/cc @glours @ndeloof

@ndeloof ndeloof enabled auto-merge (rebase) October 23, 2023 07:07
From the Go specification [1]:

  "1. For a nil slice, the number of iterations is 0."

`len` returns 0 if the slice is nil [2]. Therefore, checking
`len(v) > 0` before a loop is unnecessary.

[1]: https://go.dev/ref/spec#For_range
[2]: https://pkg.go.dev/builtin#len

Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
auto-merge was automatically disabled October 23, 2023 08:18

Head branch was pushed to by a user without write access

@Juneezee
Copy link
Contributor Author

Juneezee commented Oct 23, 2023

@ndeloof Sorry for the back and forth. Please approve the workflow one more time and it should pass now. Thank you 🙏

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.

3 participants