glide, makefile, and circleCI #3

rshriram · 2017-10-23T20:05:52Z

Makefile with some useful stuff, glide.yaml to initialize the code base.

Signed-off-by: Shriram Rajagopalan [email protected]

Signed-off-by: Shriram Rajagopalan <[email protected]>

mattklein123 · 2017-10-23T20:10:34Z

@rshriram FYI I'm going to make Circle a required check right now, pending the rest of the reviews you all are doing.

Signed-off-by: Shriram Rajagopalan <[email protected]>

rshriram · 2017-10-23T20:53:53Z

Yep thanks.

Signed-off-by: Shriram Rajagopalan <[email protected]>

This reverts commit e73ec18. Signed-off-by: Shriram Rajagopalan <[email protected]>

Signed-off-by: Shriram Rajagopalan <[email protected]>

rshriram · 2017-10-24T03:07:10Z

@kyessenov / @junr03 / @bndw PTAL. Glide setup, automatic version / SHA tagging, and circle ci setup. I also have some bits in there for proto generation using makefile, but its unused. I will leave that to @kyessenov as he is already working on bazel based pb.go generation.

rshriram · 2017-10-24T03:07:42Z

completes all the tasks in #1

davecheney · 2017-10-24T05:31:53Z

Makefile

+	@protoc --go_out=plugins=grpc:pkg/ vendor/github.com/envoyproxy/data-plane-api/api/*.proto \
+					--proto_path=vendor/github.com/envoyproxy/data-plane-api \
+					--proto_path=vendor/github.com/googleapis/googleapis/
+	@protoc --go_out=plugins=grpc:pkg/ vendor/github.com/envoyproxy/data-plane-api/api/filter/*.proto \


I don't think this is right. When I run this command I get a file with imports like this

package envoy_api_v2_filter import proto "github.com/golang/protobuf/proto" import fmt "fmt" import math "math" import envoy_api_v21 "api" import envoy_api_v22 "api" import envoy_api_v24 "api"

These should be github.com/envoyproxy/go-control-plane/pkg/envoy/api

Looks like the same issue as golang/protobuf#63

Thanks for catching this @davecheney .. I think I will take this line out for now, since @kyessenov is generating protos using bazel (using same proto version as envoy repo), and our plan is to commit these protos into the repo, so that others can just import this library.

ref envoyproxy/data-plane-api#204

bndw

Very excited to see this moving

bndw · 2017-10-24T06:42:30Z

Makefile

+tools: tools.goimports tools.golint tools.govet
+
+tools.goimports:
+	@command -v goimports >/dev/null ; if [ $$? -ne 0 ]; then \


For the Go dependencies I think the existence check can be removed and go get -u should be used

bndw · 2017-10-24T06:42:53Z

Makefile

+LDFLAGS		+= -X $(BUILD_SYM).goVersion=$(word 3,$(shell go version))
+
+.PHONY: compile
+compile:


I think this target should be renamed build

bndw · 2017-10-24T06:45:27Z

Makefile

+		echo "--> installing golint"; \
+		go get github.com/golang/lint/golint; \
+    fi
+


Can we add an installer for Glide?

tools.glide: @command -v glide 1>/dev/null ; if [ $$? -ne 0 ]; then \ echo "--> installing glide"; \ curl https://glide.sh/get | sh; \ fi

bndw · 2017-10-24T07:04:17Z

Makefile

+	@protoc --go_out=plugins=grpc:pkg/ vendor/github.com/envoyproxy/data-plane-api/api/*.proto \
+					--proto_path=vendor/github.com/envoyproxy/data-plane-api \
+					--proto_path=vendor/github.com/googleapis/googleapis/
+	@protoc --go_out=plugins=grpc:pkg/ vendor/github.com/envoyproxy/data-plane-api/api/filter/*.proto \


Looks like the same issue as golang/protobuf#63

bndw · 2017-10-24T07:06:51Z

Makefile

+
+SHELL 		:= /bin/bash
+BINDIR		:= bin
+BUILDDIR    := build


Nit -- indentation

Its GH doing the weird thing.. IT looks correct on the terminal.

Signed-off-by: Shriram Rajagopalan <[email protected]>

rshriram · 2017-10-24T13:45:12Z

@bndw updated.

Signed-off-by: Shriram Rajagopalan <[email protected]>

junr03 · 2017-10-24T17:05:04Z

.circleci/config.yml

+  build:
+    working_directory: /go/src/github.com/envoyproxy/go-control-plane
+    docker:
+      - image: rshriram/envoy-go-build:956131ed


can we pin to a generic image?

So, the standard go image from circle CI does not have protoc and glide installed. So, i had to create this. I thought that once we land this, we can (a) try to reuse lyft/envoy-build (with some improvements), or push this as a standard build image to dockerhub.

junr03 · 2017-10-24T17:14:22Z

.circleci/custom-image/install-deps.sh

@@ -0,0 +1,16 @@
+#!/bin/bash
+VERSION=3.4.0


Why are we setting this version var here? It's not used anywhere. Am I missing something?

Duh.. I hardcoded it everywhere else.. good catch

junr03 · 2017-10-24T17:16:36Z

Makefile

+BUILDDIR    := build
+DOCKERDIR	:= docker
+RELEASEDIR  := release
+OUTPUT_NAME := envoyctl


should this be go-control-plane?

envoyctl is so cool! :( .. Either way, I thought that this would change rapidly, when we start putting in CLIs in cmd/ folder.. We need one per CLI tool.

kyessenov · 2017-10-24T17:22:54Z

Makefile

+
+SHELL 		:= /bin/bash
+BINDIR		:= bin
+BUILDDIR    := build


kyessenov · 2017-10-24T17:23:51Z

glide.lock

+hash: d6fc1ffda5336bc94cc564496911ac0f6d812481c65b06e2a7833116cf3d127c
+updated: 2017-10-24T09:32:55.826752182-04:00
+imports:
+- name: github.com/envoyproxy/data-plane-api


not needed (there's no code there)

I just copied this from whatever @junr03 gave me. @junr03 any thoughts on this?

kyessenov · 2017-10-24T17:24:11Z

glide.yaml

+- package: golang.org/x/net
+  subpackages:
+  - context
+- package: github.com/envoyproxy/data-plane-api


yup, not needed in glide

Signed-off-by: Shriram Rajagopalan <[email protected]>

junr03

@rshriram did you decide to leave the glide.yaml file empty until code starts needing deps?

rshriram · 2017-10-24T19:02:20Z

Yes. That way its simpler in this PR.

update the issuer dto with certificate path

glide and makefile

cacfc46

Signed-off-by: Shriram Rajagopalan <[email protected]>

Shriram Rajagopalan added 3 commits October 23, 2017 16:13

circle ci config

f67449e

Signed-off-by: Shriram Rajagopalan <[email protected]>

tweak Makefile

716c9ee

Signed-off-by: Shriram Rajagopalan <[email protected]>

custom build image

e486681

Signed-off-by: Shriram Rajagopalan <[email protected]>

Shriram Rajagopalan added 8 commits October 23, 2017 16:55

typo

6e8fe56

Signed-off-by: Shriram Rajagopalan <[email protected]>

depend install

4446f21

Signed-off-by: Shriram Rajagopalan <[email protected]>

install glide in custom image

956131e

Signed-off-by: Shriram Rajagopalan <[email protected]>

update circle ci config

3d36cd2

Signed-off-by: Shriram Rajagopalan <[email protected]>

committing protos

e73ec18

Signed-off-by: Shriram Rajagopalan <[email protected]>

fix copyright

1f0cc53

Signed-off-by: Shriram Rajagopalan <[email protected]>

Revert "committing protos"

179137e

This reverts commit e73ec18. Signed-off-by: Shriram Rajagopalan <[email protected]>

version fix

dd6367e

Signed-off-by: Shriram Rajagopalan <[email protected]>

rshriram changed the title ~~glide and makefile~~ glide, makefile, and circleCI Oct 24, 2017

davecheney suggested changes Oct 24, 2017

View reviewed changes

bndw reviewed Oct 24, 2017

View reviewed changes

feedback

fb410af

Signed-off-by: Shriram Rajagopalan <[email protected]>

fix makefile

96979fc

Signed-off-by: Shriram Rajagopalan <[email protected]>

junr03 reviewed Oct 24, 2017

View reviewed changes

kyessenov reviewed Oct 24, 2017

View reviewed changes

Shriram Rajagopalan added 3 commits October 24, 2017 13:46

fix all

9d55505

Signed-off-by: Shriram Rajagopalan <[email protected]>

typo fix

ba270f9

Signed-off-by: Shriram Rajagopalan <[email protected]>

simplification

252a0e0

Signed-off-by: Shriram Rajagopalan <[email protected]>

junr03 reviewed Oct 24, 2017

View reviewed changes

junr03 approved these changes Oct 24, 2017

View reviewed changes

rshriram merged commit 5cf1c03 into master Oct 24, 2017

rshriram deleted the glide branch October 24, 2017 19:39

HeshanSudarshana pushed a commit to HeshanSudarshana/go-control-plane that referenced this pull request Feb 16, 2021

Merge pull request envoyproxy#3 from VirajSalaka/newbranch

f343d35

update the issuer dto with certificate path

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

glide, makefile, and circleCI #3

glide, makefile, and circleCI #3

rshriram commented Oct 23, 2017

mattklein123 commented Oct 23, 2017

rshriram commented Oct 23, 2017

rshriram commented Oct 24, 2017

rshriram commented Oct 24, 2017

davecheney Oct 24, 2017

bndw Oct 24, 2017

rshriram Oct 24, 2017

kyessenov Oct 24, 2017

bndw left a comment

bndw Oct 24, 2017

bndw Oct 24, 2017

bndw Oct 24, 2017

bndw Oct 24, 2017

bndw Oct 24, 2017

rshriram Oct 24, 2017

kyessenov Oct 24, 2017

rshriram commented Oct 24, 2017

junr03 Oct 24, 2017

rshriram Oct 24, 2017

junr03 Oct 24, 2017

rshriram Oct 24, 2017

junr03 Oct 24, 2017

rshriram Oct 24, 2017

kyessenov Oct 24, 2017

kyessenov Oct 24, 2017

rshriram Oct 24, 2017

kyessenov Oct 24, 2017

junr03 left a comment

rshriram commented Oct 24, 2017

		@@ -0,0 +1,16 @@
		#!/bin/bash
		VERSION=3.4.0

glide, makefile, and circleCI #3

glide, makefile, and circleCI #3

Conversation

rshriram commented Oct 23, 2017

mattklein123 commented Oct 23, 2017

rshriram commented Oct 23, 2017

rshriram commented Oct 24, 2017

rshriram commented Oct 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bndw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rshriram commented Oct 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

junr03 left a comment

Choose a reason for hiding this comment

rshriram commented Oct 24, 2017