Skip to content
This repository was archived by the owner on May 28, 2021. It is now read-only.

Simplify and improve the local developer experience for pushing images #167

Merged
merged 2 commits into from
Jul 5, 2018

Conversation

owainlewis
Copy link
Member

This change makes some simplifications to the Makefile around pushing development images.

Copy link

@prydie prydie left a comment

Choose a reason for hiding this comment

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

Big improvement but still more to cut 🙂

SRC_DIRS := cmd pkg test/examples
REGISTRY_STRING := $(subst /,_,$(REGISTRY))
CMD_DIRECTORIES := $(sort $(dir $(wildcard ./cmd/*/)))
Copy link

Choose a reason for hiding this comment

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

No longer used

SRC_DIRS := cmd pkg test/examples
REGISTRY_STRING := $(subst /,_,$(REGISTRY))
CMD_DIRECTORIES := $(sort $(dir $(wildcard ./cmd/*/)))
COMMANDS := $(CMD_DIRECTORIES:./cmd/%/=%)
Copy link

Choose a reason for hiding this comment

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

No longer used

Copy link
Member Author

Choose a reason for hiding this comment

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

Still used / referenced. Will remove in a subsequent PR when fixing the build step to keep things clean.

Makefile Outdated
CMD_DIRECTORIES := $(sort $(dir $(wildcard ./cmd/*/)))
COMMANDS := $(CMD_DIRECTORIES:./cmd/%/=%)
CONTAINER_FILES := $(addprefix .container-$(REGISTRY_STRING)-,$(addsuffix -$(VERSION),$(COMMANDS)))
PUSH_FILES := $(addprefix .push-$(REGISTRY_STRING)-,$(addsuffix -$(VERSION),$(COMMANDS)))

ARCH := amd64
Copy link

Choose a reason for hiding this comment

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

Should be ?=

Makefile Outdated
CMD_DIRECTORIES := $(sort $(dir $(wildcard ./cmd/*/)))
COMMANDS := $(CMD_DIRECTORIES:./cmd/%/=%)
CONTAINER_FILES := $(addprefix .container-$(REGISTRY_STRING)-,$(addsuffix -$(VERSION),$(COMMANDS)))
PUSH_FILES := $(addprefix .push-$(REGISTRY_STRING)-,$(addsuffix -$(VERSION),$(COMMANDS)))

ARCH := amd64
OS := linux
Copy link

Choose a reason for hiding this comment

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

Should be ?=

Makefile Outdated
TENANT := "spinnaker"
endif

ROOT_DIR := $(shell dirname $(abspath $(lastword $(MAKEFILE_LIST))))
PKG := github.com/oracle/mysql-operator
REGISTRY := iad.ocir.io/$(TENANT)
REGISTRY := iad.ocir.io
Copy link

Choose a reason for hiding this comment

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

Should be ?=

Makefile Outdated
@@ -18,19 +18,16 @@ ifdef WERCKER
TENANT := "oracle"
else
NEW_NAMESPACE ?= e2e-${USER}
VERSION := ${USER}-$(shell date +%Y%m%d%H%M%S)
VERSION := ${USER}-$(shell git describe --always --dirty)
Copy link

Choose a reason for hiding this comment

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

Should be ?=

Makefile Outdated
@@ -18,19 +18,16 @@ ifdef WERCKER
TENANT := "oracle"
else
NEW_NAMESPACE ?= e2e-${USER}
Copy link

Choose a reason for hiding this comment

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

No longer used

Makefile Outdated
@@ -18,19 +18,16 @@ ifdef WERCKER
TENANT := "oracle"
else
NEW_NAMESPACE ?= e2e-${USER}
VERSION := ${USER}-$(shell date +%Y%m%d%H%M%S)
VERSION := ${USER}-$(shell git describe --always --dirty)
TENANT := "spinnaker"
Copy link

Choose a reason for hiding this comment

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

Should be ?=


.PHONY: bin-clean
bin-clean:
clean:
rm -rf .go bin
Copy link

Choose a reason for hiding this comment

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

No need to rm .go as it's not created any more AFAIK

Copy link
Member Author

Choose a reason for hiding this comment

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

Still created on my machine. Will remove after fixing the build process in a subsequent PR.

Makefile Outdated
@docker push $(REGISTRY)/$*:$(VERSION)
@docker images -q $(REGISTRY)/$*:$(VERSION) > $@
push: build
@docker build --build-arg=http_proxy --build-arg=https_proxy -t $(REGISTRY)/$(TENANT)/mysql-operator:$(VERSION) -f docker/mysql-operator/Dockerfile .
Copy link

Choose a reason for hiding this comment

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

Would break these up over multiple lines

Copy link
Member Author

Choose a reason for hiding this comment

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

Agree. Split into a separate step for hygiene and broken into multiple lines.

@prydie prydie merged commit 2b5a21c into master Jul 5, 2018
@prydie prydie deleted the ol/docker-local-dev branch July 5, 2018 09:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants