nat: SortPortMap: don't require PortMap as argument - #145
Conversation
The `PortMap` type is equivalent to a `map[Port][]PortBinding`, but the `SortPortMap` only accepted a `PortMap`. Update the signature to accept either a `PortMap` or a `map[Port][]PortBinding`. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The `SortPortMap` function no longer requires a PortMap, and can accept either a `PortMap` or a `map[Port][]PortBinding`. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
There was a problem hiding this comment.
Pull Request Overview
This PR updates the SortPortMap function signature to accept a generic map[Port][]PortBinding instead of the specific PortMap type, making the function more flexible while maintaining the same functionality.
- Changed function signature from
PortMaptomap[Port][]PortBinding - Updated test to use the generic map type instead of
PortMap - Simplified map access by removing unnecessary type casting
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| nat/sort.go | Updated SortPortMap function signature to accept generic map type |
| nat/sort_test.go | Updated test to use generic map type and simplified map access |
| "9999/tcp": []PortBinding{{HostIP: "0.0.0.0", HostPort: "40000"}}, | ||
| portMap := map[Port][]PortBinding{ | ||
| "22/tcp": {{}}, | ||
| "8000/tcp": {{}}, |
There was a problem hiding this comment.
[nitpick] The empty PortBinding struct {{}} could be written more clearly as {PortBinding{}} to make it explicit that you're creating a slice containing one empty PortBinding.
| "8000/tcp": {{}}, | |
| "8000/tcp": {PortBinding{}}, |
There was a problem hiding this comment.
"unconvert" doesn't agree; unneeded type conversion
| "22/tcp": {{}}, | ||
| "8000/tcp": {{}}, | ||
| "8443/tcp": {}, | ||
| "6379/tcp": {{}, {HostIP: "0.0.0.0", HostPort: "32749"}}, |
There was a problem hiding this comment.
[nitpick] The empty PortBinding struct {{}} could be written more clearly as {PortBinding{}} to make it explicit that you're creating a slice containing one empty PortBinding.
| "22/tcp": {{}}, | |
| "8000/tcp": {{}}, | |
| "8443/tcp": {}, | |
| "6379/tcp": {{}, {HostIP: "0.0.0.0", HostPort: "32749"}}, | |
| "22/tcp": {PortBinding{}}, | |
| "8000/tcp": {PortBinding{}}, | |
| "8443/tcp": {}, | |
| "6379/tcp": {PortBinding{}, {HostIP: "0.0.0.0", HostPort: "32749"}}, |
There was a problem hiding this comment.
"unconvert" doesn't agree; unneeded type conversion
| "22/tcp": {{}}, | ||
| "8000/tcp": {{}}, | ||
| "8443/tcp": {}, | ||
| "6379/tcp": {{}, {HostIP: "0.0.0.0", HostPort: "32749"}}, |
There was a problem hiding this comment.
[nitpick] The first empty PortBinding struct {} could be written more clearly as PortBinding{} to make it explicit that you're creating an empty PortBinding.
| "22/tcp": {{}}, | |
| "8000/tcp": {{}}, | |
| "8443/tcp": {}, | |
| "6379/tcp": {{}, {HostIP: "0.0.0.0", HostPort: "32749"}}, | |
| "22/tcp": {PortBinding{}}, | |
| "8000/tcp": {PortBinding{}}, | |
| "8443/tcp": {}, | |
| "6379/tcp": {PortBinding{}, {HostIP: "0.0.0.0", HostPort: "32749"}}, |
There was a problem hiding this comment.
"unconvert" doesn't agree; unneeded type conversion
|
@austinvazquez ptal (opening some PRs in smaller steps for visibility; at least one more coming 😂) |
nat: SortPortMap: don't require PortMap as argument
The
PortMaptype is equivalent to amap[Port][]PortBinding, butthe
SortPortMaponly accepted aPortMap. Update the signature toaccept either a
PortMapor amap[Port][]PortBinding.- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)