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 to Iris version 12 and go 1.13 #78

Closed
wants to merge 1 commit into from

Conversation

kataras
Copy link
Contributor

@kataras kataras commented Nov 12, 2019

Hello @sigsevneo and @rhcarvalho, great job here.

Iris has been updated to version 12.0.1 which requires a new semantic import path. Read here for more. This PR updates the code base to be compatible with Iris v12.0.1 and support go modules with go1.13. It builds successfully (go 1.13).


Recommendation

Please rename your ./example directory to ./_examples and use a different go.mod and go.sum files for each of the web frameworks provided there, this way your library will not depend on unnecessary third-parties as _ prefix can be go-skipped.

Thanks,
Gerasimos Maropoulos. Author of the Iris Web Framework.

@rhcarvalho
Copy link
Contributor

Hi @kataras! Thanks for the PR and for the recommendation!

I've opened #79 to discuss/track your recommendation independent of this change.

@kamilogorek
Copy link
Contributor

Merged manually 749cdcc
Thanks, I'll definitely use this recommendation. Cheers!

- 1.11.x
- 1.12.x
# - 1.11.x
# - 1.12.x
Copy link
Contributor

Choose a reason for hiding this comment

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

I'll let @kamilogorek comment here. I imagine we may want to continue supporting people on earlier versions of Go.

update: commit 749cdcc was merged without the .travis.yml change.

@@ -1,50 +1,17 @@
module github.com/getsentry/sentry-go

go 1.13
Copy link
Contributor

Choose a reason for hiding this comment

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

There is some confusion around the go directive in go.mod, see golang/go#30791.

With the current state of affairs, I think we should point to the oldest release of Go supported by sentry-go, currently Go 1.11.

That's because setting a higher version could lead to build errors in dependent packages (user packages) that read like "note: module requires Go 1.13", whereas 1.11 is explicitly supported as per the documentation. See https://blog.ksub.org/bytes/2019/09/15/go.mods-go-directive/ for an example.

Copy link
Contributor Author

@kataras kataras Nov 12, 2019

Choose a reason for hiding this comment

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

Yes, it will work with >=go1.13 as explained above, you can add build tags like // +build go1.13 to the new iris files and others like // +build go1.11 to support older versions of it as well. Read more at: issue #79

Copy link
Contributor

Choose a reason for hiding this comment

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

@kataras the build tags are fine 👍 . This comment is about the go directive in the go.mod file 😉

The bump from 1.11 to 1.13 in this PR could lead to confusing output for users of sentry-go building their code with the still supported versions Go 1.11 or 1.12.

Tracking as #83.

@kamilogorek
Copy link
Contributor

Updated docs.sentry.io as well - getsentry/sentry-docs#1346

MimiDumpling pushed a commit to getsentry/sentry-docs that referenced this pull request Nov 12, 2019
Updates taken from already merged README.md update in the main repo, so no need to double-check them getsentry/sentry-go#78
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