Skip to content
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

Pass forward as R errors commands that generate Redis errors #22

Merged
merged 8 commits into from Nov 23, 2015

Conversation

@russellpierce
Copy link
Contributor

@russellpierce russellpierce commented Nov 23, 2015

Notably this will pass forward errors for any code that uses extract_reply. In particular, I was trying to address #21, which is in specific regards to exec and execv. Many functions in RcppRedis use extract_reply. However, I do not thing this change should affect them much as they are mostly protected by variable typing in their calls.

case REDIS_REPLY_ERROR: {
std::string errorMessage = std::string(reply->str);
freeReplyObject(reply);
Rcpp::stop(errorMessage);

This comment has been minimized.

@eddelbuettel

eddelbuettel Nov 23, 2015
Owner

That is a lot better than what we had!

@eddelbuettel

This comment has been minimized.

Copy link

@eddelbuettel eddelbuettel commented on dbdaa3e Nov 23, 2015

Why this commit? I long made it. Are you working a code base that is behind?

This comment has been minimized.

Copy link
Owner Author

@russellpierce russellpierce replied Nov 23, 2015

I was working on the code at the same time you were, then you merged from your local copy of my pull request. For some reason when I rebased on the upstream branch it didn't absorb this commit. In the files changed on the current pull it looks like git correctly sees that you've already applied this change.

This comment has been minimized.

Copy link

@eddelbuettel eddelbuettel replied Nov 23, 2015

Gotcha. Truly parallel is an issue. Something git merge master does the trick when you are in a branch, ditto with a fork relative to the other remote.

And most importantly the PR looked very clean in its diff. Who care about a commit count :)

eddelbuettel added a commit that referenced this pull request Nov 23, 2015
Pass forward as R errors commands that generate Redis errors
@eddelbuettel eddelbuettel merged commit 3f98a5c into eddelbuettel:master Nov 23, 2015
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.