Jumping feature #12
Jumping feature #12
Conversation
| @@ -31,6 +31,7 @@ Imports: | |||
| dplyr (>= 1.0.5), | |||
| DT (>= 0.18), | |||
| dv.manager (>= 2.0.0-17), | |||
| dv.papo (>= 2.0.1), | |||
There was a problem hiding this comment.
Do we really need this dependency?
There was a problem hiding this comment.
I just added a comment related to this suggesting the current davinci strategy for checking communication with papo.
| @@ -1,7 +1,7 @@ | |||
| Package: dv.listings | |||
| Type: Package | |||
| Title: Data listings module | |||
| Version: 4.0.1 | |||
| Version: 4.0.2 | |||
There was a problem hiding this comment.
Because of recent changes to our release process when merging to dev we should probably use a fourth component to the semantic versioning:
https://r-pkgs.org/lifecycle.html#sec-lifecycle-version-number-tidyverse
Here's an example in dv.manager:
https://github.com/Boehringer-Ingelheim/dv.manager/blob/dev/DESCRIPTION#L4
| @@ -31,6 +31,7 @@ Imports: | |||
| dplyr (>= 1.0.5), | |||
| DT (>= 0.18), | |||
| dv.manager (>= 2.0.0-17), | |||
| dv.papo (>= 2.0.1), | |||
There was a problem hiding this comment.
This hard dependency is only there for testing the communication with papo, I believe. The current way of testing communication with papo is to mock it, as we do on dv.clinlines and other modules. See, for instance:
https://github.com/Boehringer-Ingelheim/dv.clinlines/blob/main/tests/testthat/test-message_papo.R
the test_communication_with_papo function is defined here:
https://github.com/Boehringer-Ingelheim/dv.clinlines/blob/main/tests/testthat/setup.R#L22
That snippet of code is repeated across packages and I have a fix in the works to ensure that the repeat code is not an issue, so it should be OK for you to use that same strategy.
A sample app for users to review is available here: https://cn.prod.copernicus.aws.boehringer.com/content/8b149c96-cef0-4987-b9ae-2ff1b3a5e2fa