Skip to content

Conversation

@timcadman
Copy link

No description provided.

@ESCRI11 ESCRI11 self-requested a review January 12, 2026 21:56
Copy link
Contributor

@ESCRI11 ESCRI11 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides the little detail on utils.R, I think the PR looks fine and can be used as a guide for the effort of converting the other functions.

R/utils.R Outdated
if (!any(permitted_classes %in% typ)) {
msg <- glue(
"The server-side object must be of type {glue_collapse(permitted_classes, sep = ' or ')}. ",
"'{obj_name}' is type {typ}."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sometimes objects have multiple classes, which might create multiple error messages.

> a <- tibble::tibble("1")
> class(a)
[1] "tbl_df"     "tbl"        "data.frame"
> typ <- class(a)
> glue::glue("hello {typ}")
hello tbl_df
hello tbl
hello data.frame

So I think it would be a good idea to also use glue_collapse to print typ.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good spot Xavi! I've amended this, and also used the option to separate strings with more than three words to 'x, x, and x' so the message looks nicer. I've added tests to capture these eventualities.

@timcadman
Copy link
Author

I've also moved the PR template into the .github folder as R check didn't like it in root

@ESCRI11 ESCRI11 self-requested a review January 15, 2026 18:21
Copy link
Contributor

@ESCRI11 ESCRI11 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like how this looks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants