Skip to content
This repository has been archived by the owner on Feb 24, 2021. It is now read-only.

tidy up the docker build #769

Merged
merged 1 commit into from
Oct 8, 2020
Merged

tidy up the docker build #769

merged 1 commit into from
Oct 8, 2020

Conversation

chrisns
Copy link
Member

@chrisns chrisns commented Oct 7, 2020

No description provided.

@chrisns chrisns marked this pull request as ready for review October 7, 2020 22:07
TAGS="${TAGS}${DOCKER_REPO}:$(echo ${GITHUB_REF} | sed "s/refs\/tags\/v//")\n"
fi

TAGS="${TAGS}${DOCKER_REPO}:sha-${GITHUB_SHA}"
Copy link
Member

Choose a reason for hiding this comment

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

New tag?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, sorry forgot to call out

Copy link
Member

Choose a reason for hiding this comment

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

Is it used for caching?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, more for allowing someone to use something ahead of a release without opening themselves to a breaking change coming in next time they pull

Copy link
Member

Choose a reason for hiding this comment

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

And what about the 'just build on pr' part? It will push also pull requests with the format robertslando/zwave2mqtt:sha-<sha>?

Copy link
Member Author

Choose a reason for hiding this comment

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

Only prs on this repo, prs from forks don't get the secret to push

Copy link
Member

Choose a reason for hiding this comment

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

Oh missed the if in the login part

Copy link
Member Author

Choose a reason for hiding this comment

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

its more that forks don't get access to the secrets, otherwise anyone could just get your dockerhub creds

Copy link
Member

@robertsLando robertsLando left a comment

Choose a reason for hiding this comment

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

LGTM

@chrisns chrisns merged commit 6207dc4 into master Oct 8, 2020
@chrisns chrisns deleted the tidy_dockerbuild branch October 8, 2020 06:46
@chrisns
Copy link
Member Author

chrisns commented Oct 8, 2020

Seen build fail. Will fix shortly

@robertsLando
Copy link
Member

Ok let me know, sincerly I have checked the code and I don't understand why the tag isn't appended correctly in the string, maybe the : have been interpreted as a github action syntax?

@chrisns
Copy link
Member Author

chrisns commented Oct 8, 2020

its loosing that its multiline var

@chrisns
Copy link
Member Author

chrisns commented Oct 8, 2020

related to actions/toolkit#403 i think

@robertsLando
Copy link
Member

😕

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