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

Migrate to Go modules #675

Merged
merged 1 commit into from
May 24, 2019
Merged

Migrate to Go modules #675

merged 1 commit into from
May 24, 2019

Conversation

astefanutti
Copy link
Member

No description provided.

@nicolaferraro
Copy link
Member

nicolaferraro commented May 21, 2019

Great job!
Is it supposed to work if I import this repo from another repo using dep?
Currently I'm importing the Camel K API definitions in the Knative sources repo. I mean, does dep resolve transitive dependencies correctly if we move to go modules?

@astefanutti
Copy link
Member Author

I assumed Camel K wasn't consumed. It seems possible to have non-module consumers. It may be necessary to keep the Dep manifests around.

https://github.com/golang/go/wiki/Modules#providing-dependency-information-to-older-versions-of-go-and-non-module-consumers

That being said, in the specific case of the Camel K API package being imported into Knative, the transitive dependencies contraints may be better left defined by Knative anyway. So it may be just fine removing the Dep manifests altogether.

@nicolaferraro
Copy link
Member

Or, a better approach would be to make sure we don't use external dependencies in the API section, that is a good practice I've seen in other projects.

@astefanutti
Copy link
Member Author

Yes, that'd be even better. It may not be practical for the k8s APIs / primitives that we extend, though restricting to them would probably be acceptable.

@lburgazzoli
Copy link
Contributor

@nicolaferraro @astefanutti is there anything here that still need to be done ?

@nicolaferraro
Copy link
Member

I'm not sure about vendoring. Do we want to remove the vendor dir? It seems practical to me to have it synchronized there...

@astefanutti
Copy link
Member Author

astefanutti commented May 24, 2019

I would suggest we keep the Dep manifests around for a while. It seems this is the strategy some components have adopted, as the ecosystem is transitioning to Go modules. Such an example is https://github.com/kubernetes-sigs/controller-runtime.

@astefanutti
Copy link
Member Author

I've just repushed with the Gopkg.toml and Gopkg.lock left intact.

@nicolaferraro
Copy link
Member

What do you think about pushing "vendor" as we've done so far? @astefanutti , @lburgazzoli

@astefanutti
Copy link
Member Author

My understanding / opinion is that vendor should be removed:

https://github.com/golang/go/wiki/Modules#how-do-i-use-vendoring-with-modules-is-vendoring-going-away

This is also the pattern I'm seeing in the k8s ecosystem as modules are being rolled-out.

@nicolaferraro
Copy link
Member

Sounds great so!

@nicolaferraro nicolaferraro merged commit e6af27c into apache:master May 24, 2019
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.

None yet

3 participants