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

Adopt Go modules as /v3 #261

Merged
merged 9 commits into from
Aug 4, 2021
Merged

Adopt Go modules as /v3 #261

merged 9 commits into from
Aug 4, 2021

Conversation

mfridman
Copy link
Collaborator

@mfridman mfridman commented Aug 1, 2021

This introduces Go module support for goose. The intention is to keep /v3 backward-compatible with the existing tagged versions:v2.x.y and any breaking changes will roll into /v4

Up for discussion whether we want to keep the /vendor folder? For internal projects, I definitely still see vendor as useful, but for OSS projects we can probably remove. Thoughts?

@alxmsl
Copy link

alxmsl commented Aug 2, 2021

If the vendor folder is omitted, on an installation step, go tries to get latest suitable dependencies as defined in the go.mod file. And downloaded dependencies may have another versions, because a require directive asked for a minimum required version. The backward compatibility is just a recommendation for minor versions of go modules. And, unfortunately, owners don't have to guarantee a backward compatibility. So, a chance to get a dependencies hell is possible in this case ...like was with the google.golang.org/grpc@v1.30.0

I think, goose is not a library project. goose is a CLI tool, it is an end project. When users install the goose, they expect to have a tool which works the same, as it worked on testing and acceptance stages

If the vendor folder is presented in the source code, you can give this guarantee. Otherwise, no. So, I suggest to keep the vendor folder in the project.

@mfridman
Copy link
Collaborator Author

mfridman commented Aug 3, 2021

I think, goose is not a library project. goose is a CLI tool, it is an end project. When users install the goose, they expect to have a tool which works the same, as it worked on testing and acceptance stages

Thank you for the feedback, although I'd disagree with this bit.

goose is just as much a library as it is a CLI tool. Many Go users wrap the goose library to take advantage of Go-based migrations. As an example SQL + Go migrations

Most (larger) project I worked on have consumed the library to extend its functionality. Whereas smaller side-projects I use the CLI.

Based on observation, after modules and the Go module proxy were introduced more libraries are dropping /vendor and relying on the modules ecosystem.

@VojtechVitek
Copy link
Collaborator

Good points @alxmsl. Goose is both CLI and a library, as you can build custom goos binary with Go migrations. I don't have a strong opinion on this, but I'd be OK with keeping vendor/ directory too.

Copy link
Collaborator

@VojtechVitek VojtechVitek left a comment

Choose a reason for hiding this comment

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

LGTM

@mfridman
Copy link
Collaborator Author

mfridman commented Aug 4, 2021

I'll remove /vendor folder for now.

Most, if not all, of these packages are widely used in the community and are cached by the official Go module proxy.

If there is strong objection we can always bring them back via go mod vendor

@mfridman mfridman merged commit ff32c04 into master Aug 4, 2021
@mfridman mfridman deleted the go-modules branch August 4, 2021 13:16
This was referenced Aug 4, 2021
@alxmsl
Copy link

alxmsl commented Aug 5, 2021

Just one my objection was about the case when a dependency has a minor version change with a backward incompatibility.
But, of course, this issue can be solved then, when, in fact, it happen
So, I also don't mind to remove the vendor folder at this moment

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