Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add missing targets in the Makefile #4

Merged
merged 23 commits into from
May 31, 2022
Merged

Add missing targets in the Makefile #4

merged 23 commits into from
May 31, 2022

Conversation

Ruchip16
Copy link
Contributor

I have added the service.json and pattern.json files in addition to the missing targets in the makefile, check it out once and let me know the changes in case of any
@TheMosquito @t-fine

Ruchip16 and others added 3 commits April 16, 2022 00:58
@Ruchip16
Update Makefile
daa6912 
Signed-off-by: Ruchi <ruchipakhle@gmail.com>
@Ruchip16
added makefile targets
57d7bd5 
Signed-off-by: Ruchi <ruchipakhle@gmail.com>
@Ruchip16
Makfile missing targets add
2f74db6 
Signed-off-by: Ruchi <ruchipakhle@gmail.com>
t-fine
t-fine requested changes Apr 15, 2022
View reviewed changes
Makefile
@@ -1,37 +1,62 @@
# Extremely simple HTTP server that responds on port 8000 with a hello message.

DOCKERHUB_ID:=ibmosquito
DOCKER_HUB_ID ?= ibmosquito
Copy link
Contributor

Choose a reason for hiding this comment

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

@Ruchip16 with the change to DOCKER_HUB_ID make sure you change it everywhere DOCKERHUB_ID is referenced in the Makefile

Makefile Outdated
@@ -1,37 +1,62 @@
# Extremely simple HTTP server that responds on port 8000 with a hello message.

DOCKERHUB_ID:=ibmosquito
DOCKER_HUB_ID ?= ibmosquito
NAME:="web-hello-go"
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets make this SERVICE_NAME to be consistent with your other changes as well

Makefile Outdated
docker push $(DOCKERHUB_ID)/$(SERVICE_NAME):$(SERVICE_VERSION)

publish-service:
@ARCH=$(ARCH) \
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe the issue you were seeing has to do with tab/spaces. Makefiles can be pretty picky about that. Make sure all the lines below each target are one tab in. (I believe it's tabs it wants)

Makefile Outdated
hzn exchange service publish -O $(CONTAINER_CREDS) -f service.json --pull-image

publish-pattern:
@ARCH=$(ARCH) \
Copy link
Contributor

Choose a reason for hiding this comment

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

Same goes for this target

Ruchip16 and others added 12 commits April 17, 2022 10:46
@Ruchip16
Changes done
248d070
@Ruchip16
Update Makefile
ddba39c
@Ruchip16
Update Makefile
82bd458 
Signed-off-by: Ruchi <ruchipakhle@gmail.com>
@Ruchip16
added makefile targets
2fc1bc1 
Signed-off-by: Ruchi <ruchipakhle@gmail.com>
@Ruchip16
Initial content.
55ecf12 
Signed-off-by: TheMosquito <glendarling@us.ibm.com>
Signed-off-by: Ruchi <ruchipakhle@gmail.com>
@Ruchip16
Makfile missing targets add
f336d66 
Signed-off-by: Ruchi <ruchipakhle@gmail.com>
@Ruchip16
Changes done
3eac95a 
Signed-off-by: Ruchi <ruchipakhle@gmail.com>
@Ruchip16
Merge branch 'main' of https://github.com/Ruchip16/web-helloworld-go …
5353718 
…into main
@Ruchip16
Update Makefile
1951c77 
Signed-off-by: Ruchi <ruchipakhle@gmail.com>
@Ruchip16
added makefile targets
a2278c4 
Signed-off-by: Ruchi <ruchipakhle@gmail.com>
@Ruchip16
Makfile missing targets add
49d05e0 
Signed-off-by: Ruchi <ruchipakhle@gmail.com>
@Ruchip16
Changes done
b75ec82 
Signed-off-by: Ruchi <ruchipakhle@gmail.com>
@TheMosquito
Copy link
Contributor

This all looks good to me, @Ruchip16.

t-fine
t-fine requested changes Apr 18, 2022
View reviewed changes
Makefile Outdated

default: build run

build:
docker build -t $(DOCKERHUB_ID)/$(NAME):$(VERSION) .
docker build -t $(DOCKER_HUB_ID ?= ibmosquito)/$(SERVICE_NAME):$(SERVICE_VERSION) .
Copy link
Contributor

Choose a reason for hiding this comment

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

You can remove ?= ibmosquito everywhere except for line 3. It's only needed at the top to set it initially

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yess I had done that but not pushed it yet, I would do it asap

Ruchip16 and others added 6 commits April 19, 2022 18:53
@Ruchip16
Initial content.
22c4cb6 
Signed-off-by: TheMosquito <glendarling@us.ibm.com>
Signed-off-by: Ruchi <ruchipakhle@gmail.com>
@Ruchip16
Initial content.
b23859b 
Signed-off-by: TheMosquito <glendarling@us.ibm.com>
@Ruchip16
Changes done
810ab1f
@Ruchip16
Update Makefile
80f6c4e 
Signed-off-by: Ruchi <ruchipakhle@gmail.com>
@Ruchip16
Final Changes Done
9c1b613
@Ruchip16
Small Changes
9387b2f
@t-fine t-fine linked an issue May 31, 2022 that may be closed by this pull request
Add Makefile targets and horizon files #2
Closed
@t-fine t-fine linked an issue May 31, 2022 that may be closed by this pull request
Modify Makefile to allow DOCKER_HUB_ID to be overridden by env var #3
Closed
@t-fine t-fine merged commit ea99727 into open-horizon-services:main May 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@t-fine t-fine t-fine approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Modify Makefile to allow DOCKER_HUB_ID to be overridden by env var Add Makefile targets and horizon files
3 participants
@Ruchip16 @TheMosquito @t-fine