-
Notifications
You must be signed in to change notification settings - Fork 44
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
Forward compatibility with dbplyr 2.5.0 #265
Conversation
I don't _think_ there should be any other problems, but if you could run your tests with dev dbplyr installed, I'd certainly appreciate it.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #265 +/- ##
==========================================
- Coverage 34.30% 34.27% -0.04%
==========================================
Files 46 46
Lines 1775 1777 +2
==========================================
Hits 609 609
- Misses 1166 1168 +2 ☔ View full report in Codecov by Sentry. |
@@ -15,7 +15,9 @@ get_tables_from_sql.lazy_select_query <- function(query) { | |||
|
|||
#' @export | |||
get_tables_from_sql.lazy_base_remote_query <- function(query) { | |||
if (inherits(query$x, "dbplyr_table_ident")) { | |||
if (inherits(query$x, "dbplyr_table_path")) { # dbplyr >= 2.5.0 | |||
utils::getFromNamespace("table_path_name", "dbplyr")(query$x) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hadley table_path_name()
seems to be exported from dbplyr 2.5.0. Should we just use dbplyr::table_path_name()
instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can, but it will cause a R CMD check WARNING If you submit to CRAN before dbplyr 2.5.0 is released.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. I will merge this in first and modify the function call when dbplyr 2.5.0 is released.
I don't think there should be any other problems, but if you could run your tests with dev dbplyr installed, I'd certainly appreciate it.