You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
As requested I am starting to review the package (albeit a bit later than I had hoped).
I have a few things to note:
This package is required to be installed before the tests are successful. This is due to the way that targets calls nlmixr2targets. While I don't think you can change this, it does mean that you will have to install then test everytime you do a test; a simple devtools::test() will not work.
You use \(x) which may not match the minimum version of R that nlmixr2 supports (I cannot remember when it was introduced). You need to make sure that the minimum version of R is included in this package.
You will need a reference to something to smooth the wheels for the CRAN submission. Preferably a doi. I suggest the nlmixr paper and the targets paper (if it exists).
You use @return and @returns in the roxygen code; I don't think documentation cares; it is a simple inconsistency though.
There are no @examples and CRAN will probably want them.
Other than that, it seems good to me.
The text was updated successfully, but these errors were encountered:
Hi @billdenney
As requested I am starting to review the package (albeit a bit later than I had hoped).
I have a few things to note:
This package is required to be installed before the tests are successful. This is due to the way that
targets
callsnlmixr2targets
. While I don't think you can change this, it does mean that you will have to install then test everytime you do a test; a simpledevtools::test()
will not work.You use
\(x)
which may not match the minimum version of R that nlmixr2 supports (I cannot remember when it was introduced). You need to make sure that the minimum version of R is included in this package.You will need a reference to something to smooth the wheels for the CRAN submission. Preferably a doi. I suggest the nlmixr paper and the targets paper (if it exists).
You use
@return
and@returns
in the roxygen code; I don't think documentation cares; it is a simple inconsistency though.There are no
@examples
and CRAN will probably want them.Other than that, it seems good to me.
The text was updated successfully, but these errors were encountered: