Add condition to not publish for forks #21
Conversation
.circleci/config.yml
Outdated
| "$DOCKER_REPOSITORY:%" | ||
| else | ||
| echo "No tagging for forks" | ||
| fi |
There was a problem hiding this comment.
@impactmass did you have a particular reason for adding the condition on the tagging step? I don't see any harm with the tags being present if we don't push. The tags would only exist in the local build. I'd prefer to keep conditionals to a single location if possible.
There was a problem hiding this comment.
My thought was that since they aren't getting pushed the tagging can be skipped. If it's okay to have them in the local builds, I can enable. There'll be a slight modification to the tagging (because docker tagging fails tags with the / character).
.circleci/bin/docker-tags "$CIRCLE_SHA1" "$CIRCLE_BRANCH" | sed 's/\//-/g' | xargs -t -I % \
> docker-cache/docker-tags.txt
There was a problem hiding this comment.
Yep, good find! You found a bug that the acceptable branch name chars and Docker tag chars aren't the same set. Sound OK to handle that as a separate issue?
There was a problem hiding this comment.
Yes, sounds good to put that separate. I'll update this to remove the condition around tagging.
Part of #15