[GitHub] [arrow] jonkeane commented on a change in pull request #10785: ARROW-13434: [R] group_by() with an unnammed expression

[GitBox <gi...@apache.org>]

jonkeane commented on a change in pull request #10785:
URL: https://github.com/apache/arrow/pull/10785#discussion_r675547210



##########
File path: r/R/dplyr-group-by.R
##########
@@ -18,16 +18,33 @@
 
 # The following S3 methods are registered on load if dplyr is present
 
+#' @importFrom rlang quo_name

Review comment:
       My read on how we do it in the Arrow package is that we import if:
   
   * it's in the package (i.e. under the R/ directory, as opposed to in tests)
   * the dependency is _not_ a `SUGGEST`
   
   otherwise we (tend to) `pkg::` (e.g. a bunch of `rlang::eval_tidy` in `helper-expectation.R`.  I'm sure there are exceptions to this and looking for `::` in `R/` I see a few just scanning.
   
   I've seen others recommend that using a function a small number of times it's better to `pkg::` that and once some threshold is reached then switch to importing. Or use `pkg::` for the first call in a file / package to make it clear that's what one is referring to.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org