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

Interface change: use R_xlen_t for loop unrolling #731

Merged
merged 2 commits into from Jul 9, 2017

Conversation

@krlmlr
Copy link
Contributor

@krlmlr krlmlr commented Jul 8, 2017

To compile RSQLite with -Wconversion -Werror.

Copy link
Member

@eddelbuettel eddelbuettel left a comment

Looks good to me. Anybody else see issues?

@krlmlr
Copy link
Contributor Author

@krlmlr krlmlr commented Jul 8, 2017

Thanks for reviewing. Is it important to update the copyright year in the files? I have missed a few instances (also in the other PR).

@eddelbuettel
Copy link
Member

@eddelbuettel eddelbuettel commented Jul 8, 2017

I do it when I remember it (which I usuallly do) but I am also not that religious about. You can add a commit, or we clean up another time...

@kevinushey
Copy link
Contributor

@kevinushey kevinushey commented Jul 8, 2017

LGTM, although I would (very marginally) prefer to see us selecting R_xlen_t vs. int depending on whether the input SEXPTYPE supports long vectors or not. Perhaps something for a separate PR some day?

@eddelbuettel
Copy link
Member

@eddelbuettel eddelbuettel commented Jul 8, 2017

Fair point. I guess a reverse depends check will give us some clues.

@eddelbuettel
Copy link
Member

@eddelbuettel eddelbuettel commented Jul 9, 2017

Passed a full reverse-depends check with flying colour, merging.

@eddelbuettel eddelbuettel merged commit e3c8545 into RcppCore:master Jul 9, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@krlmlr krlmlr deleted the krlmlr:b-import-expression branch Mar 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.