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

Rename dockerTarget to dockerTag #851

Merged
merged 1 commit into from
Jul 24, 2016

Conversation

makubi
Copy link
Contributor

@makubi makubi commented Jul 23, 2016

Using SettingKey instead of TaskKey for the dockerTag setting.

The dockerTarget configuration is used as Docker tag and was not documented yet. Renaming it to a more applicable name.

Using 'SettingKey' instead of 'TaskKey' for the 'dockerTag' setting.
@lightbend-cla-validator

Hi @makubi,

Thank you for your contribution! We really value the time you've taken to put this together.

Before we proceed with reviewing this pull request, please sign the Typesafe Contributors License Agreement:

http://www.lightbend.com/contribute/cla

@muuki88 muuki88 merged commit 10526ed into sbt:master Jul 24, 2016
@muuki88
Copy link
Contributor

muuki88 commented Jul 24, 2016

Thanks. Looks good

@avakhrenev
Copy link

avakhrenev commented Feb 8, 2017

Hello @makubi! Why did you make dockerTag a SettingKey?

We are currently using 1.1.5 of native-packager, where we assign tags to images based on their content (hash). It allows us to skip building docker image and publishing it if image with the same tag already exists in the registry. So, our dockerTarget depends on docker:stage, which becomes impossible in 1.2.0.

Sorry if this is a wrong place to ask.

@makubi
Copy link
Contributor Author

makubi commented Feb 9, 2017

@avakhrenev I guess I just didn't think of a reason why to use a TaskKey here, but I guess you can change it again. What do you think @muuki88 ?

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.

4 participants