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

Update go.mod #107

Closed
wants to merge 1 commit into from
Closed

Update go.mod #107

wants to merge 1 commit into from

Conversation

vearutop
Copy link
Member

🤔 What's changed?

This PR changes Go module name to github.com/cucumber/messages/go/v20.

⚡️ What's your motivation?

Now that major version of this repo is v20 we need to update module path to match it.

🏷️ What kind of change is this?

  • 🐛 Bug fix (non-breaking change which fixes a defect)

(Patch release would be good to have, minor would also work).

♻️ Anything particular you want feedback on?

📋 Checklist:

  • I agree to respect and uphold the Cucumber Community Code of Conduct
  • I've changed the behaviour of the code
    • I have added/updated tests to cover my changes.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
  • Users should know about my change
    • I have added an entry to the "Unreleased" section of the CHANGELOG, linking to this pull request.

This text was originally generated from a template, then edited by hand. You can modify the template here.

@vearutop
Copy link
Member Author

Actually let me check for best practices of Go modules tagging in multi-language repos, maybe we can avoid this growth of major version and module name changes.

Copy link
Contributor

@mpkorstanje mpkorstanje left a comment

Choose a reason for hiding this comment

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

This shouldn't be done manually. All releases are made with polyglot-release to ensure a maintainer doesn't have to know the specifics for each language. So any changes should be made there.

We currently update go.mod with polyglot-release. It currently looks for and replaces a tag with the format v{major}.{minor}.{patch}. This go.mod file doesn't follow that format and because of that was ignored.

https://github.com/cucumber/polyglot-release/blob/main/polyglot-release#L108-L115

Should module names follow this v{major}.{minor}.{patch} format or should they only use v{major}?

@vearutop
Copy link
Member Author

I've checked https://go.dev/ref/mod#vcs-version, and I think the best strategy for us would be to add a tag go/v19.1.5.

https://gophers.slack.com/archives/C9BMAAFFB/p1668439716487969
image

With that, we can keep separate version history for Go module and avoid unnecessary breaking changes (keep v19 for backwards compatible changes, regardless of the global version of the repo).

It still works for me now in clean env, although it uses a v19.0.0-x pseudo version.

go get -u github.com/cucumber/messages/go/v19
go: downloading github.com/cucumber/messages v20.0.0+incompatible
go: downloading github.com/cucumber/messages/go/v19 v19.0.0-20221114144107-23c09fbac7bf
go: downloading github.com/gofrs/uuid v4.2.0+incompatible
go: downloading github.com/gofrs/uuid v4.3.1+incompatible
go: added github.com/cucumber/messages/go/v19 v19.0.0-20221114144107-23c09fbac7bf
go: added github.com/gofrs/uuid v4.3.1+incompatible

mpkorstanje added a commit to cucumber/polyglot-release that referenced this pull request Nov 14, 2022
Following on cucumber/messages#107 we can conclude that

1. only major versions should be used in go module files
2. some project will have been released with a major.minor.patch version

This change will ensure that when a new major release is made:

`module github.com/example/project/v0` will become `module github.com/example/project/v1`

and:

`module github.com/example/project/v0.X.X` will become: `module github.com/example/project/v0`.

This should fix the incorrect module names on the next major version.
mpkorstanje added a commit to cucumber/polyglot-release that referenced this pull request Nov 14, 2022
Following on cucumber/messages#107 we can conclude that

1. only major versions should be used in go module files
2. some project will have been released with a major.minor.patch version

This change will ensure that when a new major release is made:

`module github.com/example/project/v0` will become `module github.com/example/project/v1`

and:

`module github.com/example/project/v0.X.X` will become: `module github.com/example/project/v0`.

This should fix the incorrect module names on the next major version.
@mpkorstanje
Copy link
Contributor

This looks like a breaking change to me:

3e35cc4#diff-eb5b86e55ac9e362d29fea7f08860b3ff35402d3aeb9baee57c4746ef629bcc9

That aside.

We're building a repository with many languages, where some of the changes are generated. We should avoid a situation where a maintainer needs to know the specifics of versioning for each language. So while keeping v19 would be optimal in these specific circumstances and we don't have to release a fix, it is not optimal in the general case.

I've made cucumber/polyglot-release#86 to address this for future releases. This will ensure the next version of messages will be released with the v20 in the module name.

mpkorstanje added a commit to cucumber/polyglot-release that referenced this pull request Nov 14, 2022
Following on cucumber/messages#107 we can conclude that

1. only major versions should be used in go module files
2. some project will have been released with a major.minor.patch version

This change will ensure that when a new major release is made:

`module github.com/example/project/v0` will become `module github.com/example/project/v1`

and:

`module github.com/example/project/v0.X.X` will become: `module github.com/example/project/v0`.

This should fix the incorrect module names on the next major version.
mpkorstanje added a commit to cucumber/polyglot-release that referenced this pull request Nov 14, 2022
Following on cucumber/messages#107 we can conclude that

1. only major versions should be used in go module files
2. some project will have been released with a major.minor.patch version

This change will ensure that when a new major release is made:

`module github.com/example/project/v0` will become `module github.com/example/project/v1`

and:

`module github.com/example/project/v0.X.X` will become: `module github.com/example/project/v0`.

This should fix the incorrect module names on the next major version.
mpkorstanje added a commit to cucumber/polyglot-release that referenced this pull request Nov 14, 2022
Following on cucumber/messages#107 we can conclude that

1. only major versions should be used in go module files
2. some project will have been released with a major.minor.patch version

This change will ensure that when a new major release is made:

`module github.com/example/project/v0` will become `module github.com/example/project/v1`

and:

`module github.com/example/project/v0.X.X` will become: `module github.com/example/project/v0`.

This should fix the incorrect module names on the next major version.
mpkorstanje added a commit to cucumber/polyglot-release that referenced this pull request Nov 14, 2022
Following on cucumber/messages#107 we can conclude that

1. only major versions should be used in go module files
2. some project will have been released with a major.minor.patch version

This change will ensure that when a new major release is made:

`module github.com/example/project/v0` will become `module github.com/example/project/v1`

and:

`module github.com/example/project/v0.X.X` will become: `module github.com/example/project/v0`.

This should fix the incorrect module names on the next major version.
@vearutop
Copy link
Member Author

This looks like a breaking change to me:

This is indeed a breaking change, but it is unrelated to Go code.

Go library users might be annoyed by the need to upgrade their imports to sit on the latest version, while receiving no actual breaking changes. And also such import upgrades might have viral impact when another library depends on these identifiers, like for example if messages import is bumped, gherkin upgrade would also be a breaking change with new import (due to transitively exposed dependency like here).

If we could avoid unnecessary (unjustified) breaking changes, that would be kind for our users (and also maybe would make maintenance easier, as in example with gherkin).

We should avoid a situation where a maintainer needs to know the specifics of versioning for each language.

To me, this is a matter of what is more important, simplicity for maintainers or simplicity for users. As a Go library user, I really don't care about anything else that lives in this repo that isn't Go code. Go modules implementation allows having separate versioning to trade maintainer inconvenience into better user experience.

@mpkorstanje
Copy link
Contributor

Since we don't maintain two major versions in parallel, is there a reason to version the module at all? Wouldn't the git version alone be sufficient?

@otrava7
Copy link

otrava7 commented Nov 29, 2022

I think the idea is to have it versioned from the start so that whenever you break compatibility, you are forced to also change the path:
https://github.com/golang/go/wiki/Modules#semantic-import-versioning

@mpkorstanje
Copy link
Contributor

Ah. Thanks for the link. This was compelling:

Thus example.com/my/mod/mypkg is a different package than example.com/my/mod/v2/mypkg, and both may be imported in a single build, which among other benefits helps with diamond dependency problems and also allows a v1 module to be implemented in terms of its v2 replacement or vice versa.

mpkorstanje added a commit to cucumber/polyglot-release that referenced this pull request Dec 2, 2022
Following on cucumber/messages#107 we can conclude that

1. only major versions should be used in go module files
2. some project will have been released with a major.minor.patch version

This change will ensure that when a new major release is made:

`module github.com/example/project/v0` will become `module github.com/example/project/v1`

and:

`module github.com/example/project/v0.X.X` will become: `module github.com/example/project/v0`.

This should fix the incorrect module names on the next major version.
@mpkorstanje
Copy link
Contributor

mpkorstanje commented Dec 2, 2022

@vearutop I'm considering the following scenarios:

If we don't automatically update the module name for a major version. I see the following scenarios:

  • A maintainer is unfamiliar the exact semantics of Go and forget to change the module name.
    • As a result we introduce a breaking change without changing the module name.
      • We notice before the next release and put in some work to roll this back. Some effort required.
        • Users annoyed by an unannounced breaking change 🔴
      • We don't notice until after the next release, now we can't roll back easily. More effort required.
        • Users annoyed by an unannounced breaking change 🔴
        • Users annoyed by having to report a breaking change + a delay to fixing 🔴
  • We document the release process for messages with instructions to be carefully followed
    • Making a release takes too much time and effort and isn't done. 🔴

If we do automatically update the module name for a major version. I see the following scenarios:

  • Releases are made in a few minutes and actually happen 🟢
  • We update the major version without breaking changes for Go
    • Users annoyed by a non-breaking module name change. 🔴

Unfortunately there are far fewer people able and willing to maintain and contribute to an open source project then those who want to use it. Especially one that includes 8 different languages. So if given the choice between spending the rather scarce maintainer time and running the risk of accidental breaking changes, or occasionally annoying users with a non-breaking module name change I would definitely favor the maintainers time and also avoid unannounced breaking changes.

But this doesn't mean I'm happy with the current situation. Your concerns about the viral nature of these updates is warranted. It is rather unfortunate that Gherkin has a dependency on Messages. Ideally this would not be the case. Rather the Gherkin parser would create an AST and from this AST the appropriate message would be created in a Cucumber implementation. This is a problem that I've talked about in the community before but I've just written cucumber/gherkin#66 to make it permanent.

@mpkorstanje
Copy link
Contributor

The module is now set to v21

module github.com/cucumber/messages/go/v21

@mpkorstanje mpkorstanje deleted the go-mod-v20 branch December 17, 2022 13:21
@mpkorstanje
Copy link
Contributor

mpkorstanje commented Dec 17, 2022

@vearutop I've just gone through the process of releasing gherkin with a changed module version in place. I think I much better understand the problem now. Going forward we'll not change the module version.

@vearutop
Copy link
Member Author

That's great, thank you!

@mpkorstanje
Copy link
Contributor

Well, unfortunately we're not quite there yet.

We still have to check if we break the API and must upgrade the module. Does go have any automated tooling for this?

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