Exploration and additional test cases

[thebioengineer]

Hey @elimillera, I played around with your branch and made some additions that hopefully you'll like.

• I tried to simplify the template build process, changed out to use file.path() as opposed to pasting manually, and set the functions to open the created file on completion. Also, now they won't override existing files. Also now the user can manually pass a name they want in the spec as opposed to us always guessing from the username (still the default)
• some renaming of functions: ie vt_use_case -> vt_use_test_case,
• I played with a test package and id'd some bugs in the build process, so I made some edits. Main one is that the vignette process assumes the package is installed. but if building, then the package would not necessarily be installed and you could accidentally be using an old version of the package. now it is installed in a temporary library and that is the one used.
• on above I went down a path of creating a rendering function for the validation report. Now we can use the same function for rendering the validation report from either the build or installed packages.
• On the validation of an installed package, now the output file will be open in addition to being saved
• I generally added some tests

Thank you so much for doing all that work, it was great!

take a look and we can discuss edits.

I think a template for the validation report creation is still missing, and some other tooling, but I think with this addition, we are getting a good chunk closer to a point where we could solicit feedback from other users!

[elimillera]

@thebioengineer I really like the changes. The opening of the file after vt_use_<x> is a really good touch. I think the file paths were getting a little clunky and all of your changes look good to me.

[thebioengineer]

• vt_use_* will need refactoring once #16 is implemented, not blocking for now

we can just implement those changes here before merging this branch (#17)

• Wondering how we expect these templates to play with bookdown? Having naked roxygen blocks in the child Rmd files can cause problems, although this can be circumvented by using yaml header around the roxygen chunk

I think we originally looked at bookdown to facilitate the dynamic referencing, correct? do we want to continue, or intend that our markdown files require "preprocessing" to be able to be used in the context of normal markdowns

• How should users address the use case of preparing code that needs to be available both during install and post-install for validation? Does subscribing to the "validate on install" model preclude the "validate post install" or perhaps can we build it into vt_validated_build? Trying to reduce the lift when switching perspectives.

My thought was that the vt_path() function would facilitate that. Any content necessary for the validation lives in vignettes/validation (or whatever folder they designate). that contents are copied into inst/validation via vt_validated_build(). Then on install, to rerun the validation, they use vt_validate_installed_package(). vt_path() uses references that update based on the call.

• How should we consider the intersection of validation reports with usethis::use_vignette for other vignettes? This action will add .gitignore file to vignette folder.

The .gitignore is of ,R and and .html files, but definitely makes things more difficult. I assumed we would implement our own version, like vt_use_report(), that would actually be called by the setup functions, but that doesn't resolve it when the users still call usethis::use_vignette().

Coding conventions:

• why does vt_use_validation create the validation structure and not start a validation vignette? I'd like to see this renamed vt_create_validation, as a function that allows users to add validation structure to an existing location, and a new function vt_use_validation to mean initiate code via a validation report template.

I think it would make sense to implement that feature!

• Some further consideration around vt_create_* vs vt_use_* functions: when are we creating folder structures and when are we just adding files? 🤔 Can vt_create_validation add all the folders while vt_use_* add only the folder necessary for the new file?

Also reasonable. However we should still have vt_use_* create the folders if they don't exist b/c git doesnt track folders unless they have content~

• can vt_path and vt_username be pulled into an options function?

Could you elaborate? as in be able to set the values?

[elimillera]

@mariev It looks like Ellis got most of your comments but are a couple of my thoughts

I'd like to see us tighten up language around requirements and specifications. A specification is a set of requirements, where each of the files generated via vt_use_spec is a single requirement. Presumably an org could circulate a spec document for approvals/review prior to engaging in the work of writing test cases or test code.

Totally agree. I think once I can get the naming convention right for this PR it should stick for the rest of the package 😅. I think the functionality for rendering a single piece of the validation document for signatures/approval is some low hanging fruit for us.

Why are we using writeLines for content generation? imo we should be using templating features similar to VISCtemplates

usethis::use_template has been added to my package dev repertoire. 👍 I can add that in and update my PR once merging is done.

[thebioengineer]

The downside I can see to using usethis::use_template is that it expects you to using RProjects. Granted, most people are, but that can cause a sticking point for folks that are against that workflow

[mariev]

@thebioengineer it should be possible to get around the Rproject expectation in use_template by setting .here appropriately via here::set_here()

[elimillera]

@mariev Agreed. I'll merge this into my development branch and we can pick up the PR there.