pharmaverse / admiral

ADaM in R Asset Library
https://pharmaverse.github.io/admiral
Apache License 2.0
220 stars 61 forks source link

Closes #2400 Unified input date object for `derive_vars_dt()` test suite #2401

Closed zdz2101 closed 4 months ago

zdz2101 commented 5 months ago

Thank you for your Pull Request! We have developed this task checklist from the Development Process Guide to help with the final steps of the process. Completing the below tasks helps to ensure our reviewers can maximize their time on your code as well as making sure the admiral codebase remains robust and consistent.

Please check off each taskbox as an acknowledgment that you completed the task or check off that it is not relevant to your Pull Request. This checklist is part of the Github Action workflows and the Pull Request will not be merged into the main branch until you have checked off each task.

github-actions[bot] commented 5 months ago

Code Coverage

Package Line Rate Health
admiral 98%
Summary 98% (4732 / 4820)
StefanThoma commented 4 months ago

I had a look and it looks really good! I think the suppression of the warnings is fine in this context. The warnings themselves should be checked in a separate test IMO.

zdz2101 commented 4 months ago

@bms63 @StefanThoma @millerg23 Added in a separate input object that plugs in for its appropriate warnings but also checks the return object is still valid