Skip to content

Conversation

@yancyribbens
Copy link
Contributor

Add build-arg to docker for choosing which branch, tag or commit to run during build time.

@Roasbeef Roasbeef added enhancement Improvements to existing features / behaviour docker Docker-related PRs/Issues P3 might get fixed, nice to have labels Jan 21, 2019
@halseth halseth added the needs review PR needs review by regular contributors label Jan 28, 2019
@yancyribbens yancyribbens reopened this Feb 21, 2019
@halseth halseth requested a review from guggero November 7, 2019 10:22
@guggero guggero self-assigned this Nov 7, 2019
Copy link
Collaborator

@guggero guggero left a comment

Choose a reason for hiding this comment

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

The code change is definitely useful and we should have reviewed this a long time ago.

The examples might be a bit confusing as they are, I added some suggestions on how to improve them.

@yancyribbens yancyribbens force-pushed the choose-git-state-during-docker-prod-build branch from 67c7726 to bef8067 Compare November 9, 2019 13:33
@guggero guggero requested a review from carlaKC November 9, 2019 14:46
@yancyribbens yancyribbens force-pushed the choose-git-state-during-docker-prod-build branch from bef8067 to 8d31d36 Compare November 9, 2019 20:28
Copy link
Collaborator

@guggero guggero left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Collaborator

@carlaKC carlaKC left a comment

Choose a reason for hiding this comment

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

Awesome change 😻 I'm going to get some good use out of this one.
Looks good to me, and works locally.

Tiny nit on the commit name, I think it makes sense to prefix it with dockerfile: since that's where the change is (and the docs change is related to the docker file)

@yancyribbens yancyribbens force-pushed the choose-git-state-during-docker-prod-build branch from 8d31d36 to 3173680 Compare November 19, 2019 08:19
@yancyribbens
Copy link
Contributor Author

@carlaKC I see lots of example commits with docker. maybe that would be preferred over dockerfile?

@guggero
Copy link
Collaborator

guggero commented Nov 19, 2019

Yes, docker: is fine too, thanks!

@guggero guggero added this to the 0.9.0 milestone Nov 19, 2019
@guggero guggero added the v0.9.0 label Nov 19, 2019
@halseth halseth merged commit 79051ac into lightningnetwork:master Nov 19, 2019
@carlaKC
Copy link
Collaborator

carlaKC commented Nov 21, 2019

@carlaKC I see lots of example commits with docker. maybe that would be preferred over dockerfile?

Sorry I missed this, too many git emails 🙈 Thanks for the PR!
docker is good as Oli says.

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

Labels

docker Docker-related PRs/Issues enhancement Improvements to existing features / behaviour needs review PR needs review by regular contributors P3 might get fixed, nice to have

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants