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

Setting maintainer for Docker images can be optional. #390

Merged
merged 1 commit into from
Oct 27, 2014
Merged

Setting maintainer for Docker images can be optional. #390

merged 1 commit into from
Oct 27, 2014

Conversation

fiadliel
Copy link
Contributor

The maintainer line is optional in the Dockerfile. While the initial commit
assumed that it was required, it appears that this is not the case. We
can therefore skip adding the line if the maintainer is not set.

val fromCommand = Cmd("FROM", dockerBaseImage)

val maintainerCommand: Option[Cmd] = {
if (maintainer.size == 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

maintainer.isEmpty is a better choice here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

The maintainer line is optional in the Dockerfile. While the initial commit
assumed that it was required, it appears that this is not the case. We
can therefore skip adding the line if the maintainer is not set.
@muuki88 muuki88 added the docker label Oct 27, 2014
@muuki88
Copy link
Contributor

muuki88 commented Oct 27, 2014

LGTM.

muuki88 added a commit that referenced this pull request Oct 27, 2014
Setting maintainer for Docker images can be optional.
@muuki88 muuki88 merged commit 5bc1f6e into sbt:master Oct 27, 2014
@muuki88 muuki88 added the 0.8.x label Nov 26, 2014
@muuki88
Copy link
Contributor

muuki88 commented Nov 26, 2014

merged in 0.8.x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants