-
Notifications
You must be signed in to change notification settings - Fork 2.4k
v7: breaking changes you would like to see #1025
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
Comments
will pool related operation(net.Dial etc.) support context.Context?so we can record related time consumed. |
@FlamingTree do you mean passing context to Dialer? Yes, it is possible - see https://github.com/go-redis/redis/pull/1046/files |
Proper support for Go modules. |
@pierrre My main concern is that so far I don't have a single go-mod compatible import in the project I am working on (which has ~50 imports). They are either v0.9999 or incompatible. |
@vmihailenco in project that is using Go modules, you can import dependencies that don't do anything special to support Go modules. I'm currently migrating all the projects at my company:
The only issue with go-redis, it that the "update all dependencies to the latest version" command keeps reverting go-redis to v6.15.2 instead of 6.15.3, and I don't understand why... |
Try |
|
I mean it's better to have hook point on specific operation, like getConn from pool, make new connection, etc. Just like stdlib's net/http/httptrace. |
For what it's worth, Go at tip (so what will be 1.13) doesn't have the downgrade issue, probably due to golang/go#30634 and a few other module fixes. My
EDIT: This might actually be a bug in and of itself; I don't think module-aware projects should have |
I'd like to support something like this at some point, but there is nothing backwards incompatible so this change can wait until later. |
I want you to call |
@nanasi880 do you have an example of relatively popular lib that is doing this? I wonder if there are any existing conventions here. |
@vmihailenco
I think so modern library is better to propagate context.
I known only a little. It is this package is required appengine's context.
appengine's log is flush to stackdriver. If, like this
|
I'd like to see #616 done, somehow. |
https://github.com/jinzhu/gorm support logger hook. Provide a non-authoritative idea, My company's php redis lib only logging command and key , time consuming, and error. |
Is v7 likely to come out in the next few months? starting a new project and considering whether to start out on the beta versions or to stick with stable for now? |
👋 Hi @vmihailenco same question as above – I notice that v7 is still in beta, and I was updating some shared libraries we use, and was weighing up with whether to hop on the v7 train or stick to v6. |
Just tagged v7.0.0 - enjoy :) |
Perfect timing! Thanks @vmihailenco 👏 |
This issue is marked stale. It will be closed in 30 days if it is not updated. |
I am planning v7 release which should support context.Context and which will be incompatible with v6 in one or another way. So if you have in mind some other incompatible changes that make sense - you are welcome to add them here.
One such change I've just made is changing
ZAdd
fromZAdd(key string, members ...Z) *IntCmd
toZAdd(key string, members ...*Z) *IntCmd
. It saves 1 allocations and is detected by compiler - so why not.The text was updated successfully, but these errors were encountered: