Skip to content
This repository was archived by the owner on May 25, 2023. It is now read-only.

Conversation

longshorej
Copy link
Contributor

This redefines the publish in Docker task to only publish for tasks that enable the Docker plugin and define a registry.

This leads to a much better experience for lagom projects where users will have the api projects and running docker:publish would lead to many error messages for them, as it would be falling back to the default publish implementation for the API projects which is likely to fail.

@longshorej
Copy link
Contributor Author

Self merging, let me know if there's anything you think should change @fsat.

@longshorej longshorej merged commit 9f89e1f into lightbend:master Nov 19, 2017
Copy link
Contributor

@fsat fsat left a comment

Choose a reason for hiding this comment

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

I do have a question about one of the Dynamic Task. Not urgent though, just answer it when you're back.

inConfig(docker.DockerPlugin.autoImport.Docker)(
publish in docker.DockerPlugin.autoImport.Docker := {
Def.taskDyn {
dockerRepository.?.value.nonEmpty && rpDockerPublish.?.value.nonEmpty
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this statement actually do anything? If this evaluates to false, Def.task() will still be returned?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, but I agree it's confusing -- shame on me! The && is there to just short-circuit evaluation of rpDockerPublish.?.value if dockerRepository.?.value is None. rpDockerPublish.?.value will evaluate the rpDockerPublish task (which returns empty) if it's defined.

I've filed #24 to fix this up.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants