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 Modules #170

Closed
wants to merge 4 commits into from
Closed

Add Go Modules #170

wants to merge 4 commits into from

Conversation

marwan-at-work
Copy link
Contributor

@marwan-at-work marwan-at-work commented Apr 3, 2019

This PR demonstrates support for Go Modules. It doesn't have to be merged as it just shows a working version of twirp with modules. Although it can be merged if the maintainers are comfortable with the change :)

The versions in the go.mod file were generated based on the existent Gopkg.toml file. In particular, the gogo/protobuf package seems quite outdated.

Adding support for Go Modules for programs that are tagged > 1 are considered backwards incompatible and therefore they require a new version upgrade. Quite unfortunate to introduce a whole new version just because you have module support, but at the same time it ensures that people living in the old world can still function correctly by just using the previous major version.

I have not deleted the vendor folder nor the *.toml files as they would make this PR look too scary.

Updates #169

Copy link
Contributor

@spenczar spenczar left a comment

Choose a reason for hiding this comment

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

Thanks for submitting this PR! I think we have some substantially different requirements from most projects since we need to support old generated code, and I don't think we can require that our users have modules enabled. I'll cover that more over on the issue, #169.

@@ -1,10 +1,12 @@
sudo: false
language: go
go:
- 1.9.x
- 1.10.x
- 1.11.x
Copy link
Contributor

Choose a reason for hiding this comment

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

Twirp promises to support the last two minor versions, just like Go's promise with security updates. We can't stop supporting go1.10.

Copy link

Choose a reason for hiding this comment

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

Twirp promises to support the last two minor versions

Which patch versions of the minor versions? modules are only in the latest patch (1.9.7 not 1.9.6 for example) of older versions

@spenczar spenczar mentioned this pull request Apr 3, 2019
@cep21
Copy link

cep21 commented Apr 3, 2019

I believe you'll have to move everything to a sub directory and bump the major version number if you want to keep support for older versions of go in later versions of twirp.

@marwan-at-work
Copy link
Contributor Author

closing in favor of the discussion in #169 :)

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