-
Notifications
You must be signed in to change notification settings - Fork 88
Added Makefile flexibility to use in CI and dev builds #1019
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! few nits.
plugin/Makefile
Outdated
# use EXTRA_TAG for adding suffixes like "-CI" or "-RELEASE" (otional) | ||
PLUGIN_TAG ?= $(GIT_TAG)$(EXTRA_TAG) | ||
|
||
# plugin name - used as a base for ful plugin name and container for extracting rootfs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: "full" ... I guess
plugin/Makefile
Outdated
docker plugin push $(PLUGIN_NAME):$(PLUGIN_TAG) | ||
@echo Pushing $(PLUGIN_NAME):$(PLUGIN_TAG) to dockerhub.io... | ||
@docker plugin push $(PLUGIN_NAME):$(PLUGIN_TAG) || \ | ||
echo 'Please make sure the plugin is build ("make clean plugin") and you have logged in to docker hub first ("docker login -u $(DOCKER_HUB_USER)")' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about following message?
the plugin is build
=> the plugin is built
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, both typos fixed
CI failing on the known issue Merging |
d64cf05
to
218a6b7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
|
||
|
||
push: | ||
docker plugin push $(PLUGIN_NAME):$(PLUGIN_TAG) | ||
@echo Pushing $(PLUGIN_NAME):$(PLUGIN_TAG) to dockerhub.io... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: remove one redundant space
Contributes to #886
tested: manual runs and demo to @shuklanirdesh82 :-)
NOTE: Managed plugin are out of "experimental" in Docker 1.13, so it works on 1.13 or new versioning scheme (Docker '17.xx...')
//CC @shuklanirdesh82