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

Better Entrypoint Support #411

Merged
merged 3 commits into from Nov 26, 2014

Conversation

Projects
None yet
3 participants
@mhamrah
Contributor

mhamrah commented Nov 18, 2014

In order to address #410 I'd like the ability to specify entrypoint parameters. This adds a new setting, dockerEntrypoint, which takes in a Seq[String] and passes that to the ExecCmd form "ENTRYPOINT". It keeps the default bin/executableScriptName in place.

On a sidenote, I don't understand the current usage of executableScriptName. This does change the entrypoint value, but it forces the script to be places in /opt/docker/bin. It also doesn't change the outputted bin script's name. So it seems you need a specific script in a specific place in your container. I believe passing an ENTRYPOINT exec value is a more flexible approach.

Tests/documentation coming, but I wanted to get early feedback.

Thank you,

Mike

@mhamrah

This comment has been minimized.

Show comment
Hide comment
@mhamrah

mhamrah Nov 18, 2014

Contributor

I also have no idea what the deal is with the scalariform stuff, thus the two commits.

Contributor

mhamrah commented Nov 18, 2014

I also have no idea what the deal is with the scalariform stuff, thus the two commits.

@muuki88

This comment has been minimized.

Show comment
Hide comment
@muuki88

muuki88 Nov 18, 2014

Contributor

Looks like a valid use case to me (see discussion in #410 )

@fiadliel if you don't have any objections, feel free to merge.

Contributor

muuki88 commented Nov 18, 2014

Looks like a valid use case to me (see discussion in #410 )

@fiadliel if you don't have any objections, feel free to merge.

@fiadliel

This comment has been minimized.

Show comment
Hide comment
@fiadliel

fiadliel Nov 26, 2014

Contributor

LGTM (but I can't merge).

Contributor

fiadliel commented Nov 26, 2014

LGTM (but I can't merge).

@muuki88

This comment has been minimized.

Show comment
Hide comment
@muuki88

muuki88 Nov 26, 2014

Contributor

@jsuereth, @fiadliel needs merge rights :)

Contributor

muuki88 commented Nov 26, 2014

@jsuereth, @fiadliel needs merge rights :)

muuki88 added a commit that referenced this pull request Nov 26, 2014

@muuki88 muuki88 merged commit 996ac61 into sbt:master Nov 26, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment