Skip to content

Conversation

@arthurbarr
Copy link
Member

No description provided.

@arthurbarr arthurbarr requested a review from parrobe March 5, 2019 10:25
Copy link
Member

@parrobe parrobe left a comment

Choose a reason for hiding this comment

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

I can't add an in-line but the make targets for build-devserver and build-advancedserver need to call build-golang-sdk-ex not build-golang-sdk. It's because if you can the non-ex version of it for some reason make decides to try and download the sdk file a second time which if we're overriding can cause the file to be replaced with a HTTP error file.

$(DOCKER) build -t $(MQ_IMAGE_GOLANG_SDK) -f incubating/mq-golang-sdk/Dockerfile .
build-golang-sdk: docker-version build-sdk
$(info $(shell printf $(TITLE)"Build $(MQ_IMAGE_GOLANG_SDK)"$(END)))
@echo hello
Copy link
Member

Choose a reason for hiding this comment

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

debug statements

$(info $(shell printf $(TITLE)"Build $(MQ_IMAGE_GOLANG_SDK)"$(END)))
@echo hello
$(DOCKER) build --build-arg BASE_IMAGE=$(MQ_IMAGE_SDK) -t $(MQ_IMAGE_GOLANG_SDK) -f incubating/mq-golang-sdk/Dockerfile .
@echo goodbye
Copy link
Member

Choose a reason for hiding this comment

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

debug statements again

Makefile-UBUNTU Outdated
.PHONY: build-golang-sdk
build-golang-sdk:
$(DOCKER) build -t $(MQ_IMAGE_GOLANG_SDK) -f incubating/mq-golang-sdk/Dockerfile .
build-golang-sdk: docker-version build-sdk
Copy link
Member

Choose a reason for hiding this comment

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

This one should also be called build-golang-sdk-ex

We will need a separate build-golang-sdk make target that does:

.PHONY: build-golang-sdk
build-golang-sdk: downloads/$(MQ_SDK_ARCHIVE) build-golang-sdk-ex

@parrobe parrobe merged commit 0d3e177 into ibm-messaging:master Mar 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants