Skip to content
This repository has been archived by the owner on Oct 20, 2023. It is now read-only.

prepare Makefile for release #23

Merged
merged 1 commit into from
Sep 9, 2019
Merged

prepare Makefile for release #23

merged 1 commit into from
Sep 9, 2019

Conversation

michelleN
Copy link

  • add following targets: build, build-cross,
    checksum, dist, bootstrap, test

@michelleN
Copy link
Author

related to #22

Copy link
Contributor

@grampelberg grampelberg left a comment

Choose a reason for hiding this comment

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

Just a couple questions.

Makefile Outdated

.PHONY: dist
dist:
( \
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this a subshell?

Copy link
Author

Choose a reason for hiding this comment

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

Needs to be otherwise the bash script won't work correctly. Did you notice that DIST_DIRS gets calculated with the find command in this line? Dunno if that makes it clearer. I could also move this script to its own file and run it from this make target but that seems unnecessary. Open to suggestions!


.PHONY: build
build:
GOBIN=$(BINDIR) $(GO) install $(GOFLAGS) -tags '$(TAGS)' -ldflags '$(LDFLAGS)' github.com/deislabs/smi-sdk-go/cmd/installer
Copy link
Contributor

Choose a reason for hiding this comment

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

As a general style, I like separating these lines so they wrap at 80 chars (because I'm old).

Copy link
Author

Choose a reason for hiding this comment

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

sure I updated

* add following targets: build, build-cross,
checksum, dist, bootstrap, test
@grampelberg grampelberg merged commit af08faf into servicemeshinterface:master Sep 9, 2019
@michelleN michelleN deleted the makefile branch September 9, 2019 18:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants