Skip to content
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

Document list of exceptions from joining qenv objects #71

Closed
pawelru opened this issue Sep 16, 2022 · 3 comments · Fixed by #92
Closed

Document list of exceptions from joining qenv objects #71

pawelru opened this issue Sep 16, 2022 · 3 comments · Fixed by #92
Assignees
Labels

Comments

@pawelru
Copy link
Contributor

pawelru commented Sep 16, 2022

Originally posted by @pawelru in #66 (comment)

  • make it publicly available (not a private fun docs)
  • make it once only not to maintain multiple places
@nikolas-burkoff nikolas-burkoff changed the title Document list of exceptions from joining qnev objects Document list of exceptions from joining qenv objects Sep 16, 2022
@donyunardi
Copy link
Contributor

Be sure to include comments from @nikolas-burkoff
#56 (comment)
#56 (comment)
#56 (comment)

The goal is to make user aware of the exceptions and how to handle them.

@donyunardi donyunardi self-assigned this Nov 10, 2022
@donyunardi
Copy link
Contributor

donyunardi commented Nov 11, 2022

Reading through the PR comment, I believe the original request is to combine the roxygen documentation for .check_joinable function and join method into one area that is public.

We also have pre-defined rules:

  1. all private functions should have @keywords internal.
  2. private functions shouldn't be exposed in the public docs (to avoid users using private functions).
  3. internal functions not listed in pkgdown.

If we're keeping .check_joinable as internal, then how about refer the roxygen documentation for .check_joinable to the join method.

#' If two `qenv` can be joined
#'
#' Checks if two `qenv` objects can be combined.
#' For more information, please read `?join`
#'
#' @param x (`qenv`)
#' @param y (`qenv`)
#' @return `TRUE` if able to join or `character` used to print error message.
#' @keywords internal
.check_joinable <- function(x, y) {

Then under join method (public because it's exported), we explain everything.
@gogonzo what do you think?

@gogonzo
Copy link
Contributor

gogonzo commented Nov 11, 2022

@donyunardi I agree

@donyunardi donyunardi linked a pull request Nov 11, 2022 that will close this issue
donyunardi added a commit that referenced this issue Nov 17, 2022
closes #71 
move docs from check_joinable to join methods and improve documentation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants