-
-
Notifications
You must be signed in to change notification settings - Fork 7
update_ard_fmt_fn needs some love #535
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
base: main
Are you sure you want to change the base?
Conversation
Unit Tests Summary 1 files 203 suites 1m 5s ⏱️ Results for commit e71e29b. ♻️ This comment has been updated with latest results. |
Unit Test Performance DifferenceAdditional test case details
Results for commit 53fa2bc ♻️ This comment has been updated with latest results. |
ddsjoberg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is great, thank you @kimjj93 !!
I love that it begins with letting users know this is just a data frame, and any value in the ARD data frame can be updated using dplyr or base R functions. In this introduction section, can we also explain list columns (which will be new to many users), and while the list columns can be updated just like any other column in a data frame, it can be a bit trickier. because it can be somewhat less clear how to work with list columns, we have helpers, and then we go into the sections explaining the helpers?
Code Coverage SummaryDiff against mainResults for commit: e71e29b Minimum allowed coverage is ♻️ This comment has been updated with latest results |
Fixes #449
Added vignette for update_ard_fmt_fun and update_ard_stat_label under Articles
What changes are proposed in this pull request?
NEWS.md. (#, @)Provide more detail here as needed.
Reference GitHub issue associated with pull request. e.g., 'closes #'
Pre-review Checklist (if item does not apply, mark is as complete)
usethis::pr_merge_main()devtools::test_coverage()Reviewer Checklist (if item does not apply, mark is as complete)
pkgdown::build_site(). Check the R console for errors, and review the rendered website.devtools::test_coverage()When the branch is ready to be merged:
NEWS.mdwith the changes from this pull request under the heading "# cards (development version)". If there is an issue associated with the pull request, reference it in parentheses at the end update (seeNEWS.mdfor examples).Optional Reverse Dependency Checks:
Install
checkedwithpak::pak("Genentech/checked")orpak::pak("checked")