Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix docker image reference parsing by DockerImageIdentifierParser #1420

Merged
merged 1 commit into from
Jun 6, 2016

Conversation

garagatyi
Copy link

Parsing used to use reference definition from docker repo, but
fails in some very simple cases.
Rework parsing. Add comments. Add tests.

@garagatyi
Copy link
Author

@skabashnyuk @evoevodin @benoitf Please review
@riuvshin Are you OK to merge it in 4.3?

@skabashnyuk
Copy link
Contributor

ок

throw new DockerFileException("Provided image reference is invalid");
index = workingCopyOfImage.indexOf('/');
String beforeSlash = index > -1 ? workingCopyOfImage.substring(0, index) : "";
if (beforeSlash.isEmpty() || (!beforeSlash.contains(".") &&
Copy link
Contributor

@voievodin voievodin Jun 3, 2016

Choose a reason for hiding this comment

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

It is not clear from the context what these contains(...) invocations actually check, consider extracting this condition into the method or adding a comment explaining the context of the condition.

Copy link
Author

Choose a reason for hiding this comment

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

done

@voievodin
Copy link
Contributor

The solution is okay for me, please fix the comments above.

@riuvshin
Copy link
Contributor

riuvshin commented Jun 3, 2016

OK

@riuvshin riuvshin added this to the 4.3.0 milestone Jun 3, 2016
@codenvy-ci
Copy link

Build # 799 - FAILED

Please check console output at http://ci.codenvy-dev.com/jenkins/job/che-pullrequests-build/799/ to view the results.

@garagatyi garagatyi force-pushed the fixDockerImageParser branch from 1b79b01 to 02d3491 Compare June 6, 2016 07:06
Parsing used to use reference definition from docker repo, but
fails in some very simple cases.
Rework parsing. Add comments. Add tests.

Signed-off-by: Alexander Garagatyi <agaragatyi@codenvy.com>
@garagatyi garagatyi force-pushed the fixDockerImageParser branch from 02d3491 to fd9576b Compare June 6, 2016 07:21
@garagatyi garagatyi merged commit 21277ee into master Jun 6, 2016
@garagatyi garagatyi deleted the fixDockerImageParser branch June 6, 2016 07:52
@codenvy-ci
Copy link

Build # 807 - FAILED

Please check console output at http://ci.codenvy-dev.com/jenkins/job/che-pullrequests-build/807/ to view the results.

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.

5 participants