-
Notifications
You must be signed in to change notification settings - Fork 238
Conversation
Thanks for contributing @nirnanaaa - personally, I would rather that we drop the |
yeah interesing point. I think this discussion could get philosophical if we discuss vendor vs. no-vendor. I have no real opinion on it. really. I would say, if there is no need because of some private dependencies we can as well remove the vendor folder. WDYT? |
remove vendor dependencies
can someone tell my why my drone build is failing? apparently I have no permission to access the build details page. |
Hey @nirnanaaa, the errors were because Drone couldn't find any packages. Adding
to .drone.yml should fix it! |
Is there a way to enable contributors to actually see the CI? The error ofc makes sense. I forgot that. |
Unfortunately we can't give contributors access as they have to be apart of our Github organisation in order to get access to Drone. The builds are failing because the GO111MODULE environment variable also needs to be included in the "benchmarks" step. |
well, this somehow turns off any external contributor. Anyways, seems fixed now. Maybe in the future this project could consider moving to an open source CI. |
I tried PR locally,
@nirnanaaa , are you able to build the image? |
most probably not. i'll change the build to a multistaged one. |
@nirnanaaa , I think reason is |
yeah probably. I missed that. I'll create another PR after this one to refactor the dockerfile to more modern build dependencies |
RUN apk add --no-cache make | ||
WORKDIR /go/src/github.com/uswitch/kiam | ||
ADD . . | ||
RUN go mod download |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
go build
takes care of downloading the dependencies, we don't need go mod download
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was referring mostly to what kubebuilder does in caching docker layers: https://github.com/kubernetes-sigs/kubebuilder/blob/master/testdata/project-v2/Dockerfile#L8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool, TFS
@nirnanaaa as mentioned here, use |
@nirnanaaa ignore my previous comment as you have added |
but you are still right about the Dockerfile in a way that go mod download only makes sense if we copy go.mod and go.sum before the code changes. Changing that right now. let's see |
now it looks much better @surajnarwade |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🎉
do we need another approval, or something else before a merge? |
cc @rhysemmas |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for this @nirnanaaa!
as the title already hints, this PR aims to remove gopkg in favor of go modules.
Since the vendor folder get's messy pretty easily, I've split up the PR into two commits.