-
-
Notifications
You must be signed in to change notification settings - Fork 208
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
Reduce -Wconversion warnings #691
Conversation
Codecov Report
@@ Coverage Diff @@
## master #691 +/- ##
=======================================
Coverage 89.77% 89.77%
=======================================
Files 66 66
Lines 3511 3511
=======================================
Hits 3152 3152
Misses 359 359 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
But then I am on coffee #1 for the day...
Maybe let's discuss them more here on issues or on the list? Or we keep'em in branches and don't immediately PR? Other ideas? |
I could do a "for-discussion" PR in my own repo to reduce clutter here. |
LGTM |
@kevinushey @krlmlr when you have a moment, can you check package hBayesDM? Something have changed there in its dependencies, but for Rcpp 0.12.10 (as on CRAN) and our master, my builds (simple R CMD INSTALL) goes crazy and completely hogs my (big-ish) machine at work. I killed the tests by the time the load gets to 30 ... I don't think it is us. I may just skip the package going forward. |
This is presumedly with the CRAN version of |
I see the same thing on macOS -- I suspect that whatever the issue is, it's independent of Rcpp. |
by using
size_t
instead ofint
for local variables.3/n, I'm afraid the PRs after this will be more challenging to review...