Remove checks for R versions older than 3.5.0#1426
Conversation
There was a problem hiding this comment.
Pull request overview
This PR removes conditional code checks for R versions older than 3.5.0, establishing R 3.5.0 as the new baseline version requirement for the Rcpp package. This aligns with the minimum R version already tested in CI.
Key changes:
- Removed R version guards from C++ headers that checked for R >= 3.1.2, 3.2.0, and 3.5.0
- Simplified R code by removing getRversion() conditionals for R < 3.4.0 and R < 3.5.0
- Refactored cleanup script to use inline file patterns instead of loops
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/barrier.cpp | Removed R_VERSION check for DATAPTR_RO, now always using the R 3.5.0+ version |
| inst/include/Rcpp/r/headers.h | Simplified RCPP_USING_UNWIND_PROTECT definition by removing R version check |
| inst/include/Rcpp/exceptions.h | Removed conditional compilation for R_ContinueUnwind, now always available |
| inst/include/Rcpp/Symbol.h | Removed version checks for Rf_installChar availability |
| inst/include/Rcpp/Rmath.h | Removed check for dnbinom_mu family functions (available since R 3.1.2) |
| R/Attributes.R | Removed getRversion() checks for C++ plugin configuration and native routine registration |
| R/Rcpp.package.skeleton.R | Removed version checks for useDynLib registration and init.c generation |
| inst/tinytest/testRcppInterfaceUser/tests/tests.R | Removed R 3.5.0 version guard around test assertions |
| cleanup | Refactored testRcppAttributePackage cleanup from loop-based to inline patterns |
| ChangeLog | Documented the version check removals across all affected files |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * src/barrier.cpp (dataptr): Remove check for 'R (>= 3.5.0)' | ||
| * inst/include/Rcpp/Rmath.h: Remove check for 'R (>= 3.1.2)' | ||
| * inst/include/Rcpp/Symbol.h: Remove check for 'R (>= 3.2.0)' | ||
| * inst/include/Rcpp/exceptions.h: Remove check for 'R (>= 3.5.0)' | ||
| * inst/include/Rcpp/r/headers.h: Idem | ||
|
|
||
| * R/Attributes.R: Remove multiple checks for R version < 3.5.0 | ||
| * R/Rcpp.package.skeleton.R: Idem | ||
| * inst/tinytest/testRcppInterfaceUser/tests/tests.R: Remove one check |
There was a problem hiding this comment.
The cleanup script changes are not documented in the ChangeLog. While these changes appear to be a refactoring of how test package files are cleaned up (moving from loop-based to inline patterns), they should be mentioned for completeness, especially since the cleanup script is included in the PR.
| # generate native routines if we aren't using attributes (which already generate | ||
| # them automatically) and we have at least R 3.4 |
There was a problem hiding this comment.
This comment is outdated. It references "we have at least R 3.4" but this PR establishes R 3.5.0 as the baseline. The comment should be updated to either remove the version reference entirely or update it to reflect the new R 3.5.0 baseline.
| # we are running R 3.4 or higher so we can use an internal utility function | ||
| # to automatically discover additional native routines that require registration |
There was a problem hiding this comment.
This comment references "R 3.4 or higher" but this PR establishes R 3.5.0 as the baseline. The comment should be updated to reflect the new minimum version requirement or simply state that the internal utility function is available without referencing a specific version.
|
Thanks! I also poked around for a (new, added) |
|
I will wait a few hours in case any one of us changes their minds over 'Depends: R (>= 3.5.0)' but otherwise this is ready, will become 1.1.0.11 and is likely the basis for 1.1.1 come January. Yay. We got a fair number of things done this cycle. |
Closes #1425
This updates a few of the older version comparisons used in the package to provide conditional code and establishes a new baseline of R 3.5.0 -- which is also the oldest version we test in CI.
There is no formal test for R 3.5.0 or later now. I sort-of dislike declaring it in DESCRIPTION though that is the place for it. I would be fine to be more cheeky and just check in
configure(though we currently have noconfigure...). Or is that too clandestine?Checklist
R CMD checkstill passes all tests