Removed restriction that port mappings are integers to support UDP #610
Conversation
|
No problem, hope you are fine again ! Thanks a lot for your revised PR, looks good to me. The link for the contribution guidelines is here https://github.com/fabric8io/docker-maven-plugin/blob/master/CONTRIBUTING.md Actually the only thing you would need to do is a 'git commit -s --amend' on your master tree (if this is still your last commit) and then a 'git push --force' to update the PR. After this we can merge it into trunk. |
Signed-off-by: Tory McKeag <tory.mckeag@gmail.com>
| // First group is the port mapping. | ||
| // It may be split with a : in a variety of ways. | ||
| String mapping = matcher.group(1); | ||
| String[] mappingParts = mapping.split(":", 3); |
There was a problem hiding this comment.
Just re-looked at this PR and I think that is not correct here as we are talking about the ports to expose in a Dockerfile and not about mappings. So there must not be a : inlcude in the specificiation and only 8080 and 8080/tcp should be allowed.
There was a problem hiding this comment.
If you don't mind I'd fix this now, since I have to make a release right now.
| } | ||
| DockerFileKeyword.EXPOSE.addTo(b, portsS); | ||
| } | ||
| } | ||
|
|
||
| // Pattern for splitting of the protocol part of the port. | ||
| private static final Pattern PROTOCOL_SPLIT_PATTERN = Pattern.compile("(.*?)(?:/(tcp|udp))?$"); |
There was a problem hiding this comment.
Guess we can match case insensitive here, and do a lowercase when adding them to the port list. makes it a bit more flexible for users.
Signed-off-by: Roland Huß <roland@ro14nd.de>
See also #610 Signed-off-by: Roland Huß <roland@ro14nd.de>
Signed-off-by: Roland Huß <roland@ro14nd.de>
See also #610 Signed-off-by: Roland Huß <roland@ro14nd.de>
Hi @rhuss, sorry for the confusion, I've been out awhile with pneumonia. Finally got back to this, and my branch was far enough behind (and I apologize I'm kind of a Git newb) I thought it would be easier to apply to a fresh fork.
Your original link for committing guidelines doesn't work anymore, so I don't think I got everything right, other than correcting the author on the commit. Let me know if I need to do anything else.