-
Notifications
You must be signed in to change notification settings - Fork 3
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
Remove dbi.R
#50
Comments
I understand some artifacts exist because bigrquerystorage was not a CRAN package to begin with. How to avoid the incoming circular dependency thing? We could create a stripped down version of bigrquerystorage. Like an add-on to bigrquery, move bqs_auth/bqs_deauth, bqs_table_download, dbi. R to bigrquery. I am not sure of the best approach to facilitate maintenance. In the meantime, I created a branch without dbi.R |
I don't know enough to have an opinion on the other bits yet, but I'm pretty sure overriding these methods is wrong. And I think it's ok to just remove them since you don't mention them in the readme. I think it definitely makes sense for |
#57 confirms it. It has to disappear. |
Done in #52 |
I think https://github.com/meztez/bigrquerystorage/blob/main/R/dbi.R doesn't belong in bigrquerystorage, because you shouldn't be defining methods if you don't own either the generic or the class. (It also makes it difficult for me to do the right thing in bigrquery, because as soon as bigrquerystorage is loaded, my methods are overridden)
The text was updated successfully, but these errors were encountered: