Fixed signed comparison in IndexHash; unit tests for unique() #575

Merged
merged 3 commits into from Nov 5, 2016

Projects

None yet

3 participants

@nathan-russell
Contributor

This addresses the compiler warning described in #574, which is also present in SelfHash. Unit tests for the sugar function unique are also added.

@eddelbuettel
Member

Sweet. Looks good from a quick (and obviously superficial) glance.

@nathan-russell
Contributor

I actually didn't mean for these changes to get lumped into the existing PR, but hopefully it doesn't turn out to be an issue.

@eddelbuettel
Member
eddelbuettel commented Nov 4, 2016 edited

I noticed that too. No big deal -- I think you could have avoided it by starting not only from your fork but with the fork from a distinct branch.

@thirdwing
Member

LGTM. Although this can be in two separate PR.

@eddelbuettel
Member

I don't think it is worth splitting the PRs. I'll be happy to merge as is. We don't obsess too much over about squashing all PRs into single commits etc pp.

But of course if someone feels strongly enough about it to make all the changes I won't ignore them...

@eddelbuettel eddelbuettel merged commit 88ebc36 into RcppCore:master Nov 5, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment