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 from dep to Go modules #319

Merged
merged 31 commits into from
Mar 20, 2020
Merged

migrate from dep to Go modules #319

merged 31 commits into from
Mar 20, 2020

Conversation

QuentinBisson
Copy link
Contributor

Hello team, I created this as a draft as I wanted to make sure we could test it before merging.

Do you know how we could do that? The gen script works as I clone the code generator on every run.

WDYT?

@QuentinBisson QuentinBisson requested a review from a team January 16, 2020 10:48
@QuentinBisson QuentinBisson self-assigned this Jan 16, 2020
go.mod Outdated Show resolved Hide resolved
@rossf7 rossf7 mentioned this pull request Jan 28, 2020
4 tasks
@QuentinBisson
Copy link
Contributor Author

@tfussell That is awesome :) Thanks for finding that out :) I think we can change that to a true PR

Copy link
Contributor

@rossf7 rossf7 left a comment

Choose a reason for hiding this comment

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

@tfussell Nice. 🎉❤️

go.mod Show resolved Hide resolved
go.mod Show resolved Hide resolved
@tfussell
Copy link
Contributor

@tfussell Nice. 🎉❤️

Thanks. I was going to see if I could come up with some flags for generate-groups.sh that would allow me to skip the ugly cp and rm steps at the end (the output is in the scripts/ directory instead of in-place), but it's a start.

Should we target v0.17.1?

On the one hand it would be nice to do everything in a big push like with 1.16 and Helm 2.16, but on the other it would be nice to minimize the number of changes to keep this self-contained. If we could switch everything to go modules separately from updating K8s and Helm, I sort of feel like we should try to do it. Maybe we should discuss in the WG.

@MarcelMue
Copy link
Contributor

I am also in favour of moving any projects we can to go modules already. Question for me would be if importing apiextensions like this already works for dep projects?

@QuentinBisson
Copy link
Contributor Author

QuentinBisson commented Jan 29, 2020

We can vendor using go mod vendor.
So maybe we need to see if we can keep the vendor directory for dep to work while having go modules?

Otherwise, it seems people are using this PR of helm golang/dep#1963

@kopiczko
Copy link
Member

go mod vendor won't help. vendor directory of dependencies isn't used.

We can keep Gopkg.{toml,lock} for a while.

.gitignore Outdated Show resolved Hide resolved
@rossf7
Copy link
Contributor

rossf7 commented Jan 29, 2020

Another 2 cents from me. The big bang approach sucks and for me is a last resort.

We can keep Gopkg.{toml,lock} for a while.

If we can do this for dep compatibility and merge stuff sooner with K8s 1.16.6 that is fine with me. 👍

@tfussell
Copy link
Contributor

We can keep Gopkg.{toml,lock} for a while.

This seems like the way to go as long as we don't forget to keep them in sync. Maybe try to switch to go modules iteratively in 1.16 and then get rid of Gopkg.{toml,lock} in 1.17?

@kopiczko kopiczko changed the title Refactor towards go modules migrate from dep to Go modules Feb 6, 2020
Copy link
Contributor

@MarcelMue MarcelMue left a comment

Choose a reason for hiding this comment

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

I think the only thing missing here before merging is creating the release of this library as outlined by pawel here:
https://github.com/giantswarm/giantswarm/issues/9192

Copy link
Member

@kopiczko kopiczko left a comment

Choose a reason for hiding this comment

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

I'd like to have a changelog and release v0.1.0 before merging this.

@tfussell
Copy link
Contributor

tfussell commented Mar 3, 2020

Changelog is created. Waiting for a few small changes to some CRDs before creating the 0.1.0 release.

@tfussell tfussell mentioned this pull request Mar 5, 2020
@xh3b4sd
Copy link
Contributor

xh3b4sd commented Mar 20, 2020

Hi gang, I wanna push this over the finish line. Thanks for the hard work so far already. 🚀

# Conflicts:
#	CHANGELOG.md
CHANGELOG.md Outdated Show resolved Hide resolved
@rossf7
Copy link
Contributor

rossf7 commented Mar 20, 2020

@xh3b4sd Hey, I needed to merge #364 because its blocking me in app-operator and chart-operator.

I'm about to release 0.1.2 just waiting on CI.

@rossf7
Copy link
Contributor

rossf7 commented Mar 20, 2020

@xh3b4sd OK done. Thx!

# Conflicts:
#	CHANGELOG.md
CHANGELOG.md Outdated Show resolved Hide resolved
@xh3b4sd xh3b4sd merged commit c873cb9 into master Mar 20, 2020
@xh3b4sd xh3b4sd deleted the go-modules branch March 20, 2020 20:16
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.

7 participants