Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upA number of new features #16
Conversation
…pop and the interpretation of a redis (nil) as NULL rather than an error
…for the full interval
Merge remote-tracking branch 'upstream/master' Conflicts: ChangeLog
|
It looks like the integration test failed because it didn't fetch the boost headers? |
| @@ -11,6 +11,6 @@ SystemRequirements: The hiredis library (eg via package libhiredis-dev on | |||
| Debian/Ubuntu, hiredis-devel on Fedora/RedHat, or directly from source | |||
| from https://github.com/redis/hiredis) | |||
| License: GPL (>= 2) | |||
| Imports: methods, Rcpp (>= 0.11.0), RApiSerialize | |||
| Imports: methods, Rcpp (>= 0.11.0), RApiSerialize, BH | |||
eddelbuettel
Nov 22, 2015
Owner
Yes that will make Travis fail unless you also add r-cran-bh.
Yes that will make Travis fail unless you also add r-cran-bh.
eddelbuettel
Nov 22, 2015
Owner
Please add , r-cran-bh to the end of line 27, ie make it
- ./travis-tool.sh install_aptget r-cran-runit libhiredis-dev r-cran-rcpp r-cran-bh
Please add , r-cran-bh to the end of line 27, ie make it
- ./travis-tool.sh install_aptget r-cran-runit libhiredis-dev r-cran-rcpp r-cran-bh
|
Appears that lexical_cast.hpp is missing. Let me investigate. |
|
Ahh, rookie mistake, fix is here: 505d234 |
eddelbuettel
merged commit 2a63114
into
eddelbuettel:master
|
We're good now. I made a mess of the commit timeline as I fixed it all in a local branch containing your PR ... which I then didn't push back yet merged your PR. But it's all good -- PR #16 is in. Thanks!! |
|
You're welcome & thanks to you as well. |
|
Just looked at all the commits. Nice work. It's quite an avalanche though--maybe split them over smaller ones? Also, we both need to shame @richfitz into pushing something back (as he went off to the dark side and built entirely new packages: redux: Redis frontend for R requiring Redis API. |
|
I'll split them up into smaller ones next time. I saw those packages; I especially like the automatic generation of functions to match the full feature set of Redis in RedisAPI. However, there seems to be a lot of divergence in approach between what he has there and what is here in RcppRedis that would seem to mean a nightmare for integrating the two. That being said, the idea of the RedisAPI layer over the connection object seems nice; If he (or someone else) added functionality to RcppRedis or RedisAPI to allow them to work together that would seem to be a coup. I took a quick glance and I think that task is a bit beyond what I want to tackle at the moment. |
|
Thanks for splitting things up. That is a good idea. As is, generally speaking, opening an issue ticket prior to doing the work just to set a signal, allow for discussion, refinements, etc pp. And opening a ticket does not force you to do the work. But it is better (and somewhat common) to give a heads-up. Agreed on the package review. All his ideas are nice, but I too now have production depending on the RcppRedis API. And I am somewhat partial to actual packages on CRAN rather than 'promises of eventual CRAN packages' in various corners of the GitHub universe. But those minor nags from my end don't take away from what seem to be very nice packages. |
|
Thank you very much for cuing me in on the etiquette. These are among my first contributions to public projects. It looks like RedisAPI used to link against RcppRedis but then he changed what he was building on around this commit ropensci/RedisAPI@42102f9. I added an issue on RedisAPI to inquire for more details. It looks like it will really have to depend on him since the LICENSE for RedisAPI could be a problem for merging those ideas in here without a rewrite of the code. |
|
As I mentioned to Rich, I have no immediate needs beyond what RcppRedis has so I am in steady-state. I like what he is doing (eg RcppR6) but I just wish he'd contribute more to existing projects rather than reinventing (though I understand that sometimes one must). And as I said, I prefer things to be on CRAN. And lastly, I do stand my ground on using the GPL (>= 2) for my work, which happens to be the same license as R itself (which we end up linking to and creating an aggregate anyway). |
2015-11-22 Russell Pierce russell.s.pierce@gmail.com
For ease of coding and reading I used boost::lexical_cast in a couple places so that I could leverage the existing exec functions. I assume that function is efficient... but I don't know if it justifies adding the boost dependency. If not, I'd ask for kind assistance in selecting an alternative method for accomplishing the integer to string cast... I wasn't certain what would be reliable cross platform and meet style preferences.