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

Fixed return value in call to vapply in xml_integer.xml_nodeset. #147

Merged
merged 1 commit into from Dec 5, 2016

Conversation

Projects
None yet
2 participants
@ddiez
Contributor

ddiez commented Nov 15, 2016

This fixes #146. I haven't changed the unit test (mentioned in the issue) but maybe should?.

@jimhester

This comment has been minimized.

Member

jimhester commented Dec 5, 2016

You are right about the unit test, it should be using expect_identical in this case. Could you make that change, add a note to NEWS.md about the fix and thank yourself, then I can merge this.

@jimhester

This comment has been minimized.

Member

jimhester commented Dec 5, 2016

Actually I am going to make the changes myself, no need to do so. Thanks again for the PR!

@jimhester jimhester merged commit a2aefbb into r-lib:master Dec 5, 2016

3 checks passed

codecov/patch 100% of diff hit (target 77.47%)
Details
codecov/project 77.47% (+0.00%) compared to 86f8939
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ddiez

This comment has been minimized.

Contributor

ddiez commented Dec 6, 2016

Great! Thanks. Thinking about the unit test recently I don't know what would be best practice here. Use expect_equal + expect_is (for integer) or expect_identical?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment