search

deis / prototype-repo

Prototype repository for a Deis v2 component
http://deis.io
MIT License
3 stars 6 forks source link

fix(Makefile): remove redundant docker-compile build target #20

Closed arschles closed 8 years ago

krancour commented 8 years ago

I don't believe this is actually redundant. I believe it's meant to leave the possibility of compiling locally if one needs to. See deis/workflow#341

krancour commented 8 years ago

Altho, I suppose GOOS=linux GOARCH=amd64 actually negates that possibility. IMO, those should be stripped from the compile command and then the compile will naturally target the current system's architecture-- Linux within a containerized build and whatever in a local build.

arschles commented 8 years ago

Fair point. Are you opposed to naming it native-compile?

Also, in some cases it doesn't make sense to run a program outside of a container (for example, in cases where it must run in Kubernetes), so I'd like to put a comment above indicating that the target should be removed in such cases.

krancour commented 8 years ago

Well... now that you put it that way, there are probably far fewer things we'd want to compile and run on a Max (for instance) than not. The workflow client, for instance, was probably the exception and not the rule. The prototype repo probably shouldn't cater to the exception. So you have changed my mind on this. LGTM unless you think otherwise.

arschles commented 8 years ago

@krancour haha, ok! We can add it back in here as an optional target later if we find it's not a "fringe" use case later...