Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use documented R API function for extptr addr #1098

Merged
merged 2 commits into from
Jul 7, 2020

Conversation

kevinushey
Copy link
Contributor

@kevinushey kevinushey commented Jun 30, 2020

Closes #1097.

"In theory" this PR should be safe ... but I'd definitely understand if we still don't want to take it so close to release.

Checklist

  • Code compiles correctly
  • R CMD check still passes all tests
  • Prefereably, new tests were added which fail without the change
  • Document the changes by file in ChangeLog

@codecov-commenter
Copy link

codecov-commenter commented Jun 30, 2020

Codecov Report

Merging #1098 into master will increase coverage by 20.25%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           master    #1098       +/-   ##
===========================================
+ Coverage   75.35%   95.61%   +20.25%     
===========================================
  Files          64       64               
  Lines        2784     2784               
===========================================
+ Hits         2098     2662      +564     
+ Misses        686      122      -564     
Impacted Files Coverage Δ
src/api.cpp 100.00% <ø> (ø)
src/module.cpp 100.00% <ø> (ø)
R/Module.R 98.51% <0.00%> (+0.99%) ⬆️
inst/include/Rcpp/vector/proxy.h 94.11% <0.00%> (+11.76%) ⬆️
inst/include/Rcpp/internal/Proxy_Iterator.h 100.00% <0.00%> (+15.38%) ⬆️
inst/include/Rcpp/vector/string_proxy.h 100.00% <0.00%> (+20.00%) ⬆️
R/Attributes.R 99.66% <0.00%> (+22.97%) ⬆️
src/attributes.cpp 100.00% <0.00%> (+25.57%) ⬆️
R/RcppClass.R 73.75% <0.00%> (+73.75%) ⬆️
R/Rcpp.package.skeleton.R 82.85% <0.00%> (+82.85%) ⬆️
... and 1 more

@eddelbuettel
Copy link
Member

Thanks, on balance I think this will have to wait til after 1.5.0 is out.

@eddelbuettel
Copy link
Member

I rebased this against master which has the 1.0.5 release now on CRAN, and will add a new minor release version 1.0.5.1 for it.

Ok with you I hope? ;-)

@kevinushey
Copy link
Contributor Author

LGTM -- thanks!

@eddelbuettel
Copy link
Member

I was a little slow preparing a new rev.dep check for this 1.0.5.1 rc, but it is running. I may give a few hours, or maybe even wait til tomorrow, to see all is ok as expected and then merge.

@eddelbuettel
Copy link
Member

It's over 1/3 done and looking reasonable so far, so merging this now.

@eddelbuettel eddelbuettel merged commit 85f6b27 into master Jul 7, 2020
@eddelbuettel eddelbuettel deleted the bugfix/r-external-ptr-addr branch July 7, 2020 11:50
@eddelbuettel
Copy link
Member

Just for completeness: no issue came up, the reverse depends check concluded just fine, and I just commited the summary to the usual rcpp-logs repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

avoid using EXTPTR_PTR
3 participants