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

sending to cran #68

Open
1 of 5 tasks
sckott opened this issue Jul 23, 2024 · 6 comments
Open
1 of 5 tasks

sending to cran #68

sckott opened this issue Jul 23, 2024 · 6 comments
Milestone

Comments

@sckott
Copy link
Member

sckott commented Jul 23, 2024

What are things that need to be done or fixed, etc. before going to cran knowing what we know about the pain that is cran:

Tests

  • Ignore all tests on cran except a very few trivial tests
    • Alternatively we could change tests so that we use mocks. I don't think we should spend time on this approach b/c it's much better for the health of the package to have more realistic tests against localstack/minio - tools that keep up with mirroring AWS services, whereas mocks will get out of date and we'd end up with tests that don't reflect current AWS reality

Examples

Currently using a mix of @examplesIf interactive() and @examples \dontrun{ for examples ...

  • Make sure examples are NOT run during normal check; right now using examplesIf or dontrun, but we could try an approach like googledrive uses where it uses @examplesIf drive_has_token() - whereas we'd use a fxn like aws_has_creds().
    • I think we should use combination of two things, so: @examplesIf interactive() && aws_has_token() where aws_has_token checks if there is an access key, secret key, and region set. This means examples would only be checked locally in an interactive R session and with those secrets set.
  • Where possible. allow examples to run - possible being safe to run, no chance of hitting AWS itself. Probably just util fxns
    • "For the initial CRAN submission of your package, all functions must have at least one example and the example code can’t all be wrapped inside \dontrun{}"

Ideally though (insert SNL gif here) we'd actually allow examples to run more easily, BUT we'd need a way to have some kind of mock AWS account that could be freely hammered with requests, and I don't think that exists

Unfortunately there's no good way of even knowing what credentials the user has, see #71

Vignettes/README

  • Use pre-rendered vignettes. Already in place.

Other

There's sure to be other things that will come up later - this section covers all of those 😄

Dependencies

@sckott sckott added this to the v0.2 milestone Jan 10, 2025
@sckott
Copy link
Member Author

sckott commented Jan 13, 2025

@seankross Curious what you think about examples in this package. One approach is to do e.g.,

#' Are there valid AWS credentials on hand?
#'
#' @export
aws_has_creds <- function() {
  res <- tryCatch(
    paws.common::locate_credentials(),
    error = \(e) e
  )
  !inherits(res, "error")
}

#' Some exported function
#' @export
#' @examplesIf aws_has_creds()
#' run examples ...

BUT - do we think it's a good or at least okay idea to have users that run examples run them against their AWS account? Jenny does this for googledrive as an example, but I wonder if that's a bad idea to pollute their aws account. If we think it's okay, we'd have to be cautious and thoughtful about the examples we use.

thoughts?

@seankross
Copy link
Collaborator

do we think it's a good or at least okay idea to have users that run examples run them against their AWS account?

I think we should have complete examples that do not get run. Also something we need to think about in terms of "pollution" and writing examples is that anything we do in sixtyfour should be able to be undone in sixtyfour. Does that answer your question?

@sckott
Copy link
Member Author

sckott commented Jan 14, 2025

For examples that interact with AWS, we'll run examples if:

  • interactive() &&
  • aws_has_creds() &&
  • prompt that asks for confirmation

@sckott
Copy link
Member Author

sckott commented Jan 14, 2025

@seankross working on this and I don't think a prompt makes sense given the way *I think* people use examples in docs. if most people do what I do, I just copy paste code from examples and run what I want to run, not necessarily the whole thing given by the maintainer in the man page. I don't think it's a reasonable assumption that users will make sure they run a function that we make at the top of an example block that uses a prompt, but maybe I'm wrong? Maybe instead of a prompt we can have a little roxygen template we can insert into most/all exported fxn docs that's a warning about potential for polluting their account. thoughts?

@seankross
Copy link
Collaborator

I don't think a prompt makes sense given the way I think people use examples in docs...

I agree with you.

Maybe instead of a prompt we can have a little roxygen template we can insert into most/all exported fxn docs that's a warning about potential for polluting their account.

If this is not a heavy lift, let's do this instead.

@sckott
Copy link
Member Author

sckott commented Jan 17, 2025

If this is not a heavy lift, let's do this instead.

shouldn't be, can do

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants