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

Switch to go mod #225

Merged
merged 1 commit into from
May 12, 2020
Merged

Conversation

SchSeba
Copy link
Collaborator

@SchSeba SchSeba commented Apr 19, 2020

This commmit also update the kubernetes version.

Signed-off-by: Sebastian Sch sebassch@gmail.com

@ahalimx86
Copy link
Collaborator

Hi @SchSeba thanks for this PR.,
I have following two questions, maybe we need input from other contributors.

  1. Are we keeping the vendor/ and glide files for older version of Go? If so, then we may need some documentation for developer(in separate PR) when updating dependencies, we need to update both(for now, until we stop supporting vendoring altogether)

  2. What are the implication of setting device plugin API to "k8s.io/kubelet v0.18.1"? I'm concerned of compatibility with older version of K8s(v1.13+)

@SchSeba
Copy link
Collaborator Author

SchSeba commented Apr 27, 2020

Hi @ahalim-intel thanks for the comment here is my replay.

Are we keeping the vendor/ and glide files for older version of Go? If so, then we may need some documentation for developer(in separate PR) when updating dependencies, we need to update both(for now, until we stop supporting vendoring altogether)

I remove the glide files but keep the vendor folder.
I also add a new entry in the makefile deps-update that will run go mod tidy and also go mod vendor to update the vendor folder.
Not sure if this required any documentation change I search for glide in the documentation and wasn't able to find anything.

What are the implication of setting device plugin API to "k8s.io/kubelet v0.18.1"? I'm concerned of compatibility with older version of K8s(v1.13+)

This will continue to support the same versions as before because we are still using the v1beta1 API version.

@SchSeba
Copy link
Collaborator Author

SchSeba commented May 3, 2020

@ahalim-intel can you please take another look on this PR?

@ahalimx86
Copy link
Collaborator

/lgtm

@ahalimx86 ahalimx86 requested a review from zshi-redhat May 7, 2020 09:51
Makefile Outdated
@ln -nsf . vendor/src
@touch $@
.PHONY: deps-update
deps-update: $(info updating dependencies...)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing comma after deps-update: . It makes updating dependencies info always be printed when make is executed even if it's not updating dependency.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This for the comment!

I fix it :)

This commmit also update the kubernetes version.

Signed-off-by: Sebastian Sch <sebassch@gmail.com>
@zshi-redhat
Copy link
Collaborator

/lgtm

@zshi-redhat
Copy link
Collaborator

@SchSeba when generating go.mod, did you manually remove those indirect dependencies or it is not generated at all? In my local run, there are three indrect dependencies and I was using go1.12.17 with GO111MODULE set to auto.

@SchSeba
Copy link
Collaborator Author

SchSeba commented May 11, 2020

@SchSeba when generating go.mod, did you manually remove those indirect dependencies or it is not generated at all? In my local run, there are three indrect dependencies and I was using go1.12.17 with GO111MODULE set to auto.

@zshi-redhat I did not remove anything from the go.mod file manually.

I am using 1.13 with GO111MODULE set to on

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