New ways of working now the R in R-Instat is all in packages?

[rdstern]

The R-Instat developers have produced many R functions. They used to be within the R-Instat code. Now @lilyclements has collected most of them into 4 main R-Instat R packages, namely databook, instatCalculations, instatExtras and instatClimatic.

So, now the R functions are called by the R-Instat code. How should developers now work. I distinguish between 3 different tasks:

a) Developments that only change the VB code. They may call R-functions, but none are changed. Those tasks continue as before.
b) Developments that change one or more of the R functions. I suggest that then those functions return temporarily into the R-Instat code? The testing takes place without any changes needed in the 4 packages. Once the new versions are working well, then someone - usually @lilyclements will copy the new versions into the appopriate R package, delete it from the R-Instat code, and test that it works identically from there.
This step need not be done for every small change in any package. It could be (perhaps) once a month, for all changes for a new release. Occasionally a release could have a few functions in the code - in the old way.
c) Developments also needed one, or more new functions. They may adapt existing functions - giving them new names, or they may start separately.

How does that sound as a way of working. This does mean that you need to be able to get the R-code for any function, when you need it. I presume you all know the following way? I assume there may be better ones?

From the column summaries dialog script I could see it was using the summary_quantile function from databook. So I typed databook:::summary_quantile and it gives we the R code for the function in the output window.

@lilyclements how does this sound?

[rdstern]

I was in discussion with @lloyddewit who assumed a different way of working, once the packages are installed. I think "his" method would be that changes (or additions) to the functions in the R packages would always be done within the test version of that package. I assume issues would still be given in the same github site as now. I know that most R packages would have their own github version of the package, so we might too. And the test version of R-Instat might make use of those github versions. Then issues could lead to pull requests in one of those sites as well as the main R-Instat site.

This can still accommodate just one or two staff having rights to updating the packages
I look forward to others views, particularly @lilyclements and @ChrisMarsh82

[lloyddewit]

@rdstern thanks, yes I have a view (of course!) but it's not my decision. @ChrisMarsh82 and others have a better understanding of the current context and should be the ones to decide.
My view was basically to follow the process we already use when we update other external packages/libraries used by R-Instat. For example, when we need to update RInsightF461 we make the changes in the RInsightF461 repo. In the RInsightF461 repo, we make a PR that makes the required changes, and we add a unit test. We then test and merge the PR into the RInsightF461 repo main branch. We then publish a new version. The R-Instat PR just upgrades to the new RInsightF461 version (see #9454 as an example).
For the new R packages, I assume that these are (or are planned to be) in separate GitHub repos. We should make all the required changes in the R package repos; then test merge and publish a new package version (could be a minor or beta version). The R-Instat PR should just upgrade to the new R package.
We should not merge any R-Instat PRs that contain R functions. I know from experience that these functions are likely never to merge back into the R package repos, and that the R packages will diverge from R-Instat causing duplication and a maintenance headache. A developer can still of course have temporary R functions in their PR while they are developing, but these R functions should be removed before the PR is merged.

[N-thony]

@lloyddewit I really like this idea on how you update RInsightF461 and I hope @lilyclements has planned something similar for the R packages.

[lilyclements]

@lloyddewit @rdstern I saw it working as that we make the R changes in their respective packages, and we have a system there that follows the same as R-Instat: fork the repo, make changes, create a PR, get approved and merge in. I've outlined some bits on this here.

In terms of publishing to CRAN, I suggest this only happens when we make a major R-Instat release. CRAN requests you do not update a package more than once every few months.

[lloyddewit]

@N-thony @lilyclements Thanks for the comments.

... we make the R changes in their respective packages, and we have a system there that follows the same as R-Instat ...

Yes, that's exactly how I understand it as well. I also liked your 'updating R code' document.

... In terms of publishing to CRAN, I suggest this only happens when we make a major R-Instat release ...

Sounds reasonable. I assume that any R-Instat PRs that require R changes between major versions will use a minor (non-CRAN) version(s) of the R packages.

[lilyclements]

Great. I need to finish the document, but hopefully it's a useful start for now.

I believe that is all fine for us to use the development version of the R packages in R-Instat releases, as long as we are stating which package version we're using.