-
Notifications
You must be signed in to change notification settings - Fork 110
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
transition odbcDataType()
to S4
#756
Conversation
@@ -49,6 +49,7 @@ RoxygenNote: 7.3.0 | |||
SystemRequirements: GNU make, An ODBC3 driver manager and drivers. | |||
Collate: | |||
'RcppExports.R' | |||
'aaa-odbc-data-type.R' |
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.
😞
I wasn't able to find any other places where we needed a workaround to define generics before methods. We could also just delete this file and place its contents in odbc-connection.R
?
#' | ||
#' `odbcDataType.foo <- function(con, obj, ...) { | ||
#' switch_type(obj, |
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.
This example used this unexported switch_type()
helper. If we wanted to keep this example, we could export switch_type()
, but us never having received an issue related to this makes me wonder if folks ever used this code.
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.
Yeah, I don't think there's a compelling reason to keep including this example.
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.
This PR slaps. Fact. 🚫🧢.
#' | ||
#' `odbcDataType.foo <- function(con, obj, ...) { | ||
#' switch_type(obj, |
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.
Yeah, I don't think there's a compelling reason to keep including this example.
Closes #701.🐙