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.

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

add Makefile #3

Merged
merged 1 commit into from
Apr 25, 2017
Merged

add Makefile #3

merged 1 commit into from
Apr 25, 2017

Conversation

gdevillele
Copy link
Contributor

Started to write a Makefile for this repo.
This is still WIP, comments are welcome.

@dnephin Thanks for your Dockerfile! #1

Signed-off-by: Gaetan de Villele [email protected]

@dnephin
Copy link
Contributor

dnephin commented Apr 21, 2017

I think we should try and keep "targets that run inside a container" in a separate file from "targets that build images or run containers".

Anyone who wants to run the "inside a container" targets on their host can do that by pointing at the right Makefile.

Maybe we can have Makefile.go for "inside a container" targets.

@gdevillele gdevillele changed the title [WIP] add Makefile add Makefile Apr 24, 2017
@@ -2,6 +2,7 @@
FROM golang:1.8-alpine

RUN apk add -U git
RUN apk add -U make
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like formatting is off a bit here because of tabs vs spaces

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thx! On it.

Makefile Outdated

# build the cli
build:
go build -o ./build/docker ./cmd/docker
Copy link
Contributor

@dnephin dnephin Apr 24, 2017

Choose a reason for hiding this comment

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

To build all the binaries we can do this:

    gox -output build/docker-{{.OS}}-{{.Arch}}
        -osarch="linux/arm linux/amd64 darwin/amd64 windows/amd64"
        ./cmd/docker

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! Used it for the "cross" target :)

@gdevillele gdevillele mentioned this pull request Apr 24, 2017
Copy link
Contributor

@dnephin dnephin left a comment

Choose a reason for hiding this comment

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

One more small comment, and I would be good to squash to a single commit

@@ -2,6 +2,7 @@
FROM golang:1.8-alpine

RUN apk add -U git
RUN apk add -U make
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be one line:

RUN   apk add -U git make

Copy link
Contributor Author

Choose a reason for hiding this comment

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

on it :)

Copy link
Contributor

@dnephin dnephin left a comment

Choose a reason for hiding this comment

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

LGTM with a squash

@gdevillele gdevillele merged commit b37ea22 into docker:master Apr 25, 2017
@gdevillele gdevillele deleted the add-makefile branch April 25, 2017 16:58
@thaJeztah thaJeztah added this to the 17.06.0 milestone Jul 21, 2017
vdemeester pushed a commit to vdemeester/docker-cli that referenced this pull request Oct 13, 2017
Add deploy command, kubernetes version
thaJeztah added a commit that referenced this pull request Oct 4, 2021
[20.10 backport] Update Go to 1.16.8
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.

3 participants