-
Notifications
You must be signed in to change notification settings - Fork 555
Conversation
Hi, Thanks a lot for the PR, saves me some time and I appreciate a lot :) There is apparently an issue with the build on Mac+clang. I checked on my phone and I think it might be because is not included in redis_error.hpp. I can't check for now, but will try to fix that tonight if you can't do it in the meantime. As for tacopie, you can do a PR too (I'm the owner of the library), that would be awesome! Thanks again :) |
I did a PR on tacopie also, everything looks good. For me, seems this error is coming from tacopie (because it's not fixed yet). I'm not sure how you will manage this dependency issue. Not exactly related, but tacopie is a dependency since 3.0.0, right?
Maybe you should update your README.md. |
Ok, I did have a look. The compilation error on mac+clang was fixed by adding the Moreover, I also got some warnings on mac+clang concerning the usage of AFter some searches, did find a relevant post about it: So I updated the PR to use the right syntax (aka Travis looks fine with the changes, so I merge: do not hesitate to come back to me if there is still any issue. I also merged PR on tacopie, making the same changes concerning ATOMIC_VAR_INIT. Thanks again a loooot for this, that's really cool! Have a nice one :) |
Hello @Cylix, thanks for your time and the library, but seems we have a problem 😕 I'm doing some tests here now and I'll let you know. If I get to some workaround, I'll do another PR, ok? --- EDIT --- I have submitted another PR for tacopie and cpp_redis. []'s |
Thanks for your time too, I really appreciate :) |
I confirm the resolution |
@Cylix and @simpuc please take a look about what was being discussed on issue #37.
I just made a little change on the constructor (explict keyword) and included another constructor.
All tests seems to pass...
PS: To this fix works properly tacopie also needs some fix for the std::atomic issues. Should I do a pull request there too?