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

Add go.mod #103

Closed
wants to merge 2 commits into from
Closed

Add go.mod #103

wants to merge 2 commits into from

Conversation

vladimiroff
Copy link

As stated in golang/go#24384:

vgo only allows v2+ when there is a go.mod file so please tell the package maintainer/owner to make the package vgo compatible.

Thus it's impossible to add anything other than version 1.0 of this package as dependency in project built with vgo.

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.

This looks good to me. How do we test this?

(I haven't used vgo for anything real yet.)

@vladimiroff
Copy link
Author

@VojtechVitek in order to make sure this change is working I've renamed the project in my fork, tagged a relase with go.mod and tried importing it. Even after the latest changes in go.mod file, I still can't make this work.

Just opened golang/go#25494 will follow it and apply fixes in this branch (if necessary) before declaring this ready to be merged :)

@vladimiroff vladimiroff changed the title Add go.mod [WIP] Add go.mod May 22, 2018
As vgo proposes from version v2.0.0 there's supposed to be a /v2 in the
module name. For users this means that they have to import goose as
`github.com/pressly/goose/v2`. For just using the goose binary, nothing
changes. See: https://blog.golang.org/versioning-proposal

At some point (CL: 105215) vgo started supporting and preferring
unquoted strings in mod files. This change unquotes everything.
@vladimiroff vladimiroff changed the title [WIP] Add go.mod Add go.mod May 23, 2018
@vladimiroff
Copy link
Author

This took less than expected. There are two proposed CLs that fix the reported issue. You could see how to test (and use my fork for the purpose) in the issue's description.

The PR is now tested and now ready to be merged :)

@@ -0,0 +1,8 @@
module github.com/pressly/goose/v2
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need /v2 ?

Does the consumer of this package need to reference "v2" in all import statements too?

Copy link
Author

Choose a reason for hiding this comment

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

If there's no /v2 in the module name, vgo can use either tip of master or latest v1.. release. Yes, all the import statements will need this /v2, too.

When developers reach v2, semantic import versioning means that a /v2/ is added to the import path at the end of the module root prefix: my/thing/v2/sub/pkg. There are good reasons for this convention, as described in the earlier post, but it is still a departure from existing tools. Realizing this, vgo will not use any v2 or later tag in a source code repository without first checking that it has a go.mod with a module path declaration ending in that major version (for example, module "my/thing/v2"). Vgo uses that declaration as evidence that the author is using semantic import versioning to name packages within that module. This is especially important for multi-package modules, since the import paths within the module must contain the /v2/ element to avoid referring back to the v1 module.

-- https://research.swtch.com/vgo-module

Choose a reason for hiding this comment

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

Goose don't have version tag eat. Please add tag v0.1.0 or v1.0.0 and use module module github.com/pressly/goose

Copy link
Collaborator

Choose a reason for hiding this comment

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

This repo has git tags. Latest is v2.3.0.

Choose a reason for hiding this comment

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

Oh. Sorry. Yes, vgo want major version in module.

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.

This would be a breaking pkg path change.

I prefer sticking with v2.3.0+incompatible until we release v3.0.

A similar approach was taken in many v2+ packages out there on Github.

@eaglemoor
Copy link

eaglemoor commented Oct 10, 2018

Ok. Maybe in v3 will realise #63 )
Good task for change major version.

@VojtechVitek
Copy link
Collaborator

@eaglemoor very much possible. We actually came up with an interesting approach internally.

Use timestamps in development branch, and make it a serial numbers on git merge (via simple merge bot).

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