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

rrlite #6

Closed
richfitz opened this Issue Mar 31, 2015 · 23 comments

Comments

Projects
None yet
8 participants
@richfitz
Member

richfitz commented Mar 31, 2015

    1. What does this package do? (explain in 50 words or less)
      This package provides an interface to "rlite" - a standalone, zero-configuration port of the Redis interface (rlite is to Redis what sqlite is to MySQL).
    1. Paste the full DESCRIPTION file inside a code block (bounded by ``` on either end).
Package: rrlite
Title: R bindings to rlite
Version: 0.1.0
Description: R bindings to rlite.  rlite is a "self-contained,
  serverless, zero-configuration, transactional redis-compatible
  database engine. rlite is to Redis what SQLite is to SQL.".
Depends: R (>= 3.1.2)
License: BSD_2_clause + file LICENSE
LazyData: true
Author: Rich FitzJohn
Maintainer: Rich FitzJohn <rich.fitzjohn@gmail.com>
Suggests: testthat,
    knitr,
    RcppRedis
Imports: R6, Rcpp
LinkingTo: Rcpp
VignetteBuilder: knitr
SystemRequirements: GNU make
    1. URL for the package (the development repository, not a stylized html page)
      https://github.com/richfitz/rrlite
    1. What data source(s) does it work with (if applicable)?
      User-supplied, but arbitrary. Special support for data.frames. No direct support for existing remote data sources.
    1. Who is the target audience?
      Package developers, technically minded users who need to process large data or scale analyses. This is a low-level package.
    1. Check the box next to each policy below, confirming that you agree. These are mandatory.
  • The repository has continuous integration with Travis and/or another service
  • The package contains a vignette
  • The package contains a reasonably complete readme with devtools install instructions
  • The package contains unit tests
    1. Do you agree to follow the rOpenSci packaging guidelines? These aren't mandatory, but we strongly suggest you follow them. If you disagree with anything, please explain.
  • Are there any packages that your package depends on that are not on CRAN?
  • Do you intend for this package to go on CRAN?
  • Does the package have a CRAN accepted license?
  • Did devtools::check() produce any errors or warnings? If so paste them below.
* checking for executable files ... WARNING
Found the following executable files:
  src/rlite/deps/lua/src/lua
  src/rlite/deps/lua/src/luac

(this can be fixed by uncommenting the cleanup script but that slows down local development and I've not worked out a good workflow there. Rbuildignore has proved difficult to use for this).

    1. Please add explanations below for any exceptions to the above:
      Not covered in the requested information, but in it's current state, the package is highly unlikely to work under Windows. I'm holding off attempting with R this until the upstream rlite is confirmed to work. Once that's done, this package would definitely want appveyor support to ensure continued Windows compatibility.

@richfitz richfitz referenced this issue Mar 31, 2015

Closed

rrlite #1

@sckott

This comment has been minimized.

Show comment
Hide comment
@sckott

sckott Apr 1, 2015

Member

hey @jeroenooms

@richfitz package is our first to go through this process & acts as a test case to work out bugs before we start sending packages through the process. Do you mind reviewing this package? We'll check things that can be automated, including things that Ripley gets pissed about. Started a list of things for reviewers to check in the wiki https://github.com/ropensci/onboarding/wiki/For-Reviewers

@karthik @cboettig @richfitz @jennybc thoughts on the things for reviewers to check in the wiki?

Member

sckott commented Apr 1, 2015

hey @jeroenooms

@richfitz package is our first to go through this process & acts as a test case to work out bugs before we start sending packages through the process. Do you mind reviewing this package? We'll check things that can be automated, including things that Ripley gets pissed about. Started a list of things for reviewers to check in the wiki https://github.com/ropensci/onboarding/wiki/For-Reviewers

@karthik @cboettig @richfitz @jennybc thoughts on the things for reviewers to check in the wiki?

@karthik

This comment has been minimized.

Show comment
Hide comment
@karthik

karthik Apr 1, 2015

Member

@sckott The list is a good start. The last bit might be hard for a reviewer to judge since we haven't articulated that ourselves in policies. I've been taking a crack at that we should proceed with this one and answer questions as they arise. We can use that to keep iterating on that wiki.

Member

karthik commented Apr 1, 2015

@sckott The list is a good start. The last bit might be hard for a reviewer to judge since we haven't articulated that ourselves in policies. I've been taking a crack at that we should proceed with this one and answer questions as they arise. We can use that to keep iterating on that wiki.

@jeroen

This comment has been minimized.

Show comment
Hide comment
@jeroen

jeroen Apr 2, 2015

Contributor

Sure, I'll have a look at it next week.

Contributor

jeroen commented Apr 2, 2015

Sure, I'll have a look at it next week.

@gaborcsardi

This comment has been minimized.

Show comment
Hide comment
@gaborcsardi

gaborcsardi Apr 2, 2015

IMO a lot of these can be automated, so why don't we write a checkr package that does the automated parts, and then shows a nice summary. Parts of R CMD check can be used I guess, although its code quite involved and thus lengthy and somewhat unclear (to me).

gaborcsardi commented Apr 2, 2015

IMO a lot of these can be automated, so why don't we write a checkr package that does the automated parts, and then shows a nice summary. Parts of R CMD check can be used I guess, although its code quite involved and thus lengthy and somewhat unclear (to me).

@richfitz

This comment has been minimized.

Show comment
Hide comment
@richfitz

richfitz Apr 2, 2015

Member

Ideally, because each package is on travis we can poll to get the most recent build logs? That gives all the R CMD check output without having to rerun anything. I agree that the more that is automated the better.

However, the items on the "for reviewers" checklist are subjective and not easily automatable. The intent here is that new packages to ropensci go beyond just working and are following and evolving set of best practices.

Member

richfitz commented Apr 2, 2015

Ideally, because each package is on travis we can poll to get the most recent build logs? That gives all the R CMD check output without having to rerun anything. I agree that the more that is automated the better.

However, the items on the "for reviewers" checklist are subjective and not easily automatable. The intent here is that new packages to ropensci go beyond just working and are following and evolving set of best practices.

@karthik

This comment has been minimized.

Show comment
Hide comment
@karthik

karthik Apr 2, 2015

Member

However, the items on the "for reviewers" checklist are subjective and not easily automatable. The intent here is that new packages to ropensci go beyond just working and are following and evolving set of best practices.

💯 to Rich's point. Nearly everything that needs to be automated already has. We need human oversight on fit and code quality.

Member

karthik commented Apr 2, 2015

However, the items on the "for reviewers" checklist are subjective and not easily automatable. The intent here is that new packages to ropensci go beyond just working and are following and evolving set of best practices.

💯 to Rich's point. Nearly everything that needs to be automated already has. We need human oversight on fit and code quality.

@stewid

This comment has been minimized.

Show comment
Hide comment
@stewid

stewid Apr 10, 2015

Member

@richfitz @sckott @karthik @cboettig @jennybc

Could the code be DRYed out? Is there a lot of code duplication?**

No

Are there user interface improvements that could be made?

No, the user interface was clearly described in the vignettes and intuitive to use.

Are there performance improvements that could be made?

No

Is documentation (installation instructions/vignettes/examples/demos) adequate?

The documentation is adequate with installation instructions that are easy to follow, two good vignettes and examples to all methods.

Does the package seem like a good fit for rOpenSci?

Yes 👍 It seems like a very efficient package to work with key-value data.

Other comments:

rrlite and git2r has chosen completly different strategies how to include and update the source code of the underlying interface. It would be interesting to have a follow up and list pros and cons with each approach for future guidance and best practice.

Member

stewid commented Apr 10, 2015

@richfitz @sckott @karthik @cboettig @jennybc

Could the code be DRYed out? Is there a lot of code duplication?**

No

Are there user interface improvements that could be made?

No, the user interface was clearly described in the vignettes and intuitive to use.

Are there performance improvements that could be made?

No

Is documentation (installation instructions/vignettes/examples/demos) adequate?

The documentation is adequate with installation instructions that are easy to follow, two good vignettes and examples to all methods.

Does the package seem like a good fit for rOpenSci?

Yes 👍 It seems like a very efficient package to work with key-value data.

Other comments:

rrlite and git2r has chosen completly different strategies how to include and update the source code of the underlying interface. It would be interesting to have a follow up and list pros and cons with each approach for future guidance and best practice.

@richfitz richfitz referenced this issue Apr 10, 2015

Open

Prepare for rOpenSci onboarding #4

5 of 7 tasks complete
@richfitz

This comment has been minimized.

Show comment
Hide comment
@richfitz

richfitz Apr 10, 2015

Member

Thanks, @stewid! 🌟

With packaging sources - I agree that there are pros and cons (there's a third approach in jqr that seems less robust than either). The git2r compilation also looks really different (https://github.com/ropensci/git2r/blob/master/src/Makevars.in)

Perhaps we could draft a blog post on "including 3rd party software in a package" and include best practices about listing authors, etc?

Member

richfitz commented Apr 10, 2015

Thanks, @stewid! 🌟

With packaging sources - I agree that there are pros and cons (there's a third approach in jqr that seems less robust than either). The git2r compilation also looks really different (https://github.com/ropensci/git2r/blob/master/src/Makevars.in)

Perhaps we could draft a blog post on "including 3rd party software in a package" and include best practices about listing authors, etc?

@stewid

This comment has been minimized.

Show comment
Hide comment
@stewid

stewid Apr 10, 2015

Member

I'm using the Makefile https://github.com/ropensci/git2r/blob/master/Makefile#L62 to 1) copy files from a local libgit2 repository to git2r, 2) apply patches e.g. to pass R CMD check, and 3) create Makevars.in and Makevars.win from an R script https://github.com/ropensci/git2r/blob/master/scripts/build_Makevars.r

Member

stewid commented Apr 10, 2015

I'm using the Makefile https://github.com/ropensci/git2r/blob/master/Makefile#L62 to 1) copy files from a local libgit2 repository to git2r, 2) apply patches e.g. to pass R CMD check, and 3) create Makevars.in and Makevars.win from an R script https://github.com/ropensci/git2r/blob/master/scripts/build_Makevars.r

@karthik

This comment has been minimized.

Show comment
Hide comment
@karthik

karthik Apr 10, 2015

Member

Perhaps we could draft a blog post on "including 3rd party software in a package" and include best practices about listing authors, etc?

💯 👏

Member

karthik commented Apr 10, 2015

Perhaps we could draft a blog post on "including 3rd party software in a package" and include best practices about listing authors, etc?

💯 👏

@karthik

This comment has been minimized.

Show comment
Hide comment
@karthik

karthik Apr 12, 2015

Member

@stewid If there isn't anything more for @richfitz to resolve, ok for us to move this into ropensci?
cc @sckott

Member

karthik commented Apr 12, 2015

@stewid If there isn't anything more for @richfitz to resolve, ok for us to move this into ropensci?
cc @sckott

@stewid

This comment has been minimized.

Show comment
Hide comment
@stewid

stewid Apr 12, 2015

Member

Yes

Member

stewid commented Apr 12, 2015

Yes

@karthik

This comment has been minimized.

Show comment
Hide comment
@karthik

karthik Apr 12, 2015

Member

ok great.
@richfitz Since you do have ownership of ropensci, go ahead and transfer it when you get a chance.
Other things to check off:

  • Add a rOpenSci footer.
  • Add an appropriate entry into ropensci.org/packages/index.html
Member

karthik commented Apr 12, 2015

ok great.
@richfitz Since you do have ownership of ropensci, go ahead and transfer it when you get a chance.
Other things to check off:

  • Add a rOpenSci footer.
  • Add an appropriate entry into ropensci.org/packages/index.html
@richfitz

This comment has been minimized.

Show comment
Hide comment
@richfitz

richfitz Apr 12, 2015

Member

Sure - I was just going to squash the issues raised over here first.

One thing though; I was thinking about factoring out the auto-generated API (that'd be the "Intermediate" and "High" level interfaces in the rrlite readme) into a new package RedisAPI and leaving rrlite as a driver for rrlite. That would make the interface work more smoothly with RcppRedis and rredis. Would it be OK to transfer both halves into rOpenSci?

Member

richfitz commented Apr 12, 2015

Sure - I was just going to squash the issues raised over here first.

One thing though; I was thinking about factoring out the auto-generated API (that'd be the "Intermediate" and "High" level interfaces in the rrlite readme) into a new package RedisAPI and leaving rrlite as a driver for rrlite. That would make the interface work more smoothly with RcppRedis and rredis. Would it be OK to transfer both halves into rOpenSci?

@karthik

This comment has been minimized.

Show comment
Hide comment
@karthik

karthik Apr 12, 2015

Member

Right, missed that. Makes sense to do that first.

Would it be OK to transfer both halves into rOpenSci?

Sounds ok to me. @sckott any objections?

Member

karthik commented Apr 12, 2015

Right, missed that. Makes sense to do that first.

Would it be OK to transfer both halves into rOpenSci?

Sounds ok to me. @sckott any objections?

@jeroen

This comment has been minimized.

Show comment
Hide comment
@jeroen

jeroen Apr 12, 2015

Contributor

Perhaps you can add a configure script with:

git submodule init
git submodule update

so dependencies will be automatically pulled in if not already? Do you plan to bundle a frozen version of the rlite source code when publishing to CRAN?

Contributor

jeroen commented Apr 12, 2015

Perhaps you can add a configure script with:

git submodule init
git submodule update

so dependencies will be automatically pulled in if not already? Do you plan to bundle a frozen version of the rlite source code when publishing to CRAN?

@richfitz

This comment has been minimized.

Show comment
Hide comment
@richfitz

richfitz Apr 12, 2015

Member

Yes - I'll grab the current point of the rlite sources for CRAN. Probably once rlite stabilises I'll shift to pointing at its releases (right now it's a moving target).

Member

richfitz commented Apr 12, 2015

Yes - I'll grab the current point of the rlite sources for CRAN. Probably once rlite stabilises I'll shift to pointing at its releases (right now it's a moving target).

@sckott

This comment has been minimized.

Show comment
Hide comment
@sckott

sckott Apr 13, 2015

Member

no objections to putting both in ropensci, sounds good

Member

sckott commented Apr 13, 2015

no objections to putting both in ropensci, sounds good

@richfitz richfitz referenced this issue Apr 13, 2015

Closed

meta questions #2

@richfitz

This comment has been minimized.

Show comment
Hide comment
@richfitz

richfitz Apr 13, 2015

Member

I've transferred ownership, though there a couple of remaining things in ropensci/rrlite#4

Member

richfitz commented Apr 13, 2015

I've transferred ownership, though there a couple of remaining things in ropensci/rrlite#4

@richfitz richfitz closed this Apr 13, 2015

@cboettig

This comment has been minimized.

Show comment
Hide comment
@cboettig

cboettig Apr 13, 2015

Member

👏 🚀

Member

cboettig commented Apr 13, 2015

👏 🚀

@sckott

This comment has been minimized.

Show comment
Hide comment
@sckott

sckott Apr 13, 2015

Member

welcome on 🐗 d

Member

sckott commented Apr 13, 2015

welcome on 🐗 d

@karthik

This comment has been minimized.

Show comment
Hide comment
@karthik

karthik Apr 14, 2015

Member

@sckott you're 🔪ing me

Member

karthik commented Apr 14, 2015

@sckott you're 🔪ing me

@sckott

This comment has been minimized.

Show comment
Hide comment
@sckott

sckott Apr 14, 2015

Member

:p

Member

sckott commented Apr 14, 2015

:p

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment