Skip to content
This repository was archived by the owner on Dec 1, 2025. It is now read-only.

Conversation

@roopakv
Copy link
Contributor

@roopakv roopakv commented Aug 18, 2021

This updates x/sys for the sdk and therefor adds support for go 1.17. More info here Homebrew/homebrew-core#83413

I didn't forget about

  • Tests
  • Changelog
  • Documentation

Closes #???

@roopakv
Copy link
Contributor Author

roopakv commented Aug 25, 2021

@mRrvz woould you be able to merge this and cut a new release

@mRrvz
Copy link
Contributor

mRrvz commented Aug 25, 2021

Will it not be enough just to merge? Why do you want a release? @roopakv

@roopakv
Copy link
Contributor Author

roopakv commented Aug 25, 2021

It helps not pinning to a commit on brew. Even a patch helps

@Totktonada
Copy link
Contributor

(I'm not experienced in Go, so, please, don't take my questions as nitpicking.)

Why replace is used, not just a line in the require section?

What also doubts me: now we should keep in the mind to don't forget to update x/sys from time to time, right? Can we require x/sys >= some_good_version?

I'm not sure it really should be fixed on Go modules side (it looks as a problem of distributions like homebrew), but I don't mind to workaround it anyway.

For the history sake: the original issue is golang/go#45702.

@mRrvz
Copy link
Contributor

mRrvz commented Aug 30, 2021

@roopakv please answer the question above

@roopakv
Copy link
Contributor Author

roopakv commented Aug 30, 2021

@Totktonada

replace because x/sys is a transitive dependency so require wont work, replace works for nested deps.

cannot do x >= with go. The team will need to remove this later

This is not a problem on homebrew, but a problem in x/sys and go 1.17. cartridge-cli needs this change to work with go 1.17

@Totktonada
Copy link
Contributor

Thank you for the clarification. I don't mind the change.

@mRrvz mRrvz merged commit 0698ed0 into tarantool:master Aug 30, 2021
@roopakv roopakv deleted the roopak/xsys branch August 30, 2021 20:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants