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

Changed a few DBI methods to dispatch over the first two arguments #57

Merged
merged 2 commits into from Mar 9, 2018

Conversation

@bborgesr
Copy link
Contributor

@bborgesr bborgesr commented Dec 7, 2017

(as per the default definition in DBI)

@krlmlr, thanks for letting me know about this issue. Are the changes here enough to close #55?
(To be on the safe side, I also went ahead and made the same change to other functions in this file that also have a default implementation in DBI with double dispatch.)

…nd `dbReadTable()` to dispatch over the first two arguments, as per the default definition in DBI (closes #55).
@krlmlr
Copy link

@krlmlr krlmlr commented Dec 7, 2017

Thanks. To test this, can you please run R CMD check with the dev version of DBI? When do you plan to release to CRAN?

@bborgesr
Copy link
Contributor Author

@bborgesr bborgesr commented Dec 8, 2017

@krlmlr, I don't get any errors/warnings/messages on devtools::check() for pool, no matter which branch I'm in (master or dispatch), even if I'm on the dev version of DBI... So, it seems like you are catching something I can't replicate.

I released pool somewhat recently and I didn't have any plans for another release soon. But if this is a breaking change, I can do another release, np. When are you planning on releasing DBI?

@bborgesr bborgesr merged commit 2b39676 into master Mar 9, 2018
3 checks passed
3 checks passed
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@bborgesr bborgesr deleted the dispatch branch Mar 9, 2018
@krlmlr
Copy link

@krlmlr krlmlr commented Mar 10, 2018

Thanks for looking into it! DBI 0.8 is on CRAN, I mentioned that I contacted you regarding the check problems in pool.

@bborgesr
Copy link
Contributor Author

@bborgesr bborgesr commented Mar 10, 2018

Thanks @krlmlr, I also submitted pool 0.1.4 to CRAN yesterday and am now waiting for it to be accepted.

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.

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