Add coverage in more places for unicode/*.jl #12767
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kshyatt
added
test
|
Tests have passed, could I beg you to merge my coverage improvements @kshyatt? |
|
This is not important at all but just a thought. Maybe it is best to avoid commit messages like "Test for last uncovered line in utf32.jl" and "Add tests for complete coverage of checkstring," because it could be argued that these type of messages are talking about things which are "out of scope" for what the PR actually does. For example, if someone merged some new functions into these files before this is merged then the commit messages would suddenly be false. |
|
Reasonable point, I'll take that into account next PR. |
|
(or if somebody does change any of these, I'll rebase with a reword) |
StefanKarpinski
added a commit
that referenced
this pull request
Aug 24, 2015
Add coverage in more places for unicode/*.jl
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added test for uncovered line in utf32.jl
Changed Uint to UInt in types.jl
Removed a duplicate convert method in utf16.jl
Add test to cover converting a long utf8 sequence in utf8.jl
Changed a method in checkstring.jl for better coverage
Added more tests for checkstring.jl, test accept_long_char option