Thanks for doing this.
toggle quoted messageShow quoted text
A few comments:
- Having files with the same name but in different directories can get
confusing. It'd be good to have distinct names.
- I think the changes to Rinterface.h and Rembedded.h may cause
problems with code that embeds R, e.g. RStudio. Looking at the
functions, there's not much benefit to making them private, so I'd
recommend leaving them alone.
- From Rinternals.h, we might want to keep the following functions for
use in packages:
MAYBE_SHARED, NO_REFERENCES etc.
R_RegisterFinalizerEx, R_MakeWeakRefC are referenced in 'Writing R
Extensions', so should probably be kept.
- I'd also recommend looking closely at the functions required by the
packages in src/library. Some of them probably should be kept.
Other functions and variables from Rinternals.h that I think we should
look into making private at time permits:
MISSING / SET_MISSING
HASHTAB / SET_HASHTAB
On Fri, Jan 13, 2017 at 5:52 AM, Lukas Stadler <lukas.stadler@...> wrote:
Happy new year to you all!
One outcome of the discussions in this group and last year’s RIOT workshop
is that the native API deserves a cleanup, moving out things that aren’t
needed as a first step.
My colleague Zbynek took the time to go over the whole API and move all
functionality that is not used into separate header files that are not part
of the public include directory.
He was able to move out 183 functions, and all packages from CRAN (that we
were able to build without special setup) work fine with the changes.
The changes are available as a PR on github:
github also provides a raw diff:
I’d like to gather feedback before suggesting this change to r-devel:
- do you think there’s a better approach than having separate header files
in include/private? our goal was to keep the changes to the main codebase
- are there functions or macros that shouldn’t be moved although they are
currently not used by any packages on CRAN?
- are there functions or macros that this patch doesn’t move but we should
still try to move, although this will require fixes in some packages?
The patch is not set in stone, we’re happy to make any changes (or perform
other tasks) that will make it easier for r-devel to accept these changes.
For some code-related comments it may be simpler to post on github (because
you can add comments to specific lines in the changes).
Rconsortium-wg-api mailing list