-
Notifications
You must be signed in to change notification settings - Fork 0
Merge test to main #2
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
Conversation
Initial GitHub release
Add validation to ensure results dataset is not empty
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.
Looks very good! Only one questions, see comments.
Stable Release
- Update the version number and make sure -9000 suffix is removed.
- Merge all NEWS under a single version entry
- Integration tests pass
- devtools::document() is up to date
- devtools::check() runs internally
DESCRIPTION
Outdated
| RoxygenNote: 7.3.2 | ||
| Remotes: | ||
| Boehringer-Ingelheim/dv.manager@main, | ||
| Boehringer-Ingelheim/dv.papo@main |
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.
@mingstat Why do we list dv.papo here under "Remotes"? I cannot remember that we did this for other modules that are able to jump to papo. Does this create any benefit?
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.
The dv.papo dependency is optional. For consistency with other modules, I’ve removed it from DESCRIPTION to keep the dependency list clean.
No description provided.