[
https://issues.apache.org/jira/browse/DERBY-2760?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12542286
]
Dag H. Wanvik commented on DERBY-2760:
--------------------------------------
Patch looks good and ran cleanly.
Some comments:
- Test had some magic numbers that could be explained:
is there some threshold or not to test "large" here?
- some missing javadoc for public methods.
- The skipping of 2 characters from the ReaderToUTF8Stream were not obvious;
the format in that module could have been better documented, but not part of
this patch, I know.
- A typo in indentifier name: testSkipFullyOnInalidStreamCJK
I did not check coverage more than superficially here since I am not familiar
with the module under test.
+1
> Clean-up issues for UTF8Util.java
> ---------------------------------
>
> Key: DERBY-2760
> URL: https://issues.apache.org/jira/browse/DERBY-2760
> Project: Derby
> Issue Type: Improvement
> Components: JDBC
> Affects Versions: 10.3.1.4
> Reporter: Knut Anders Hatlen
> Assignee: Kristian Waagan
> Priority: Trivial
> Fix For: 10.4.0.0
>
> Attachments: derby-2760-1a-remove_unused_method.diff,
> derby-2760-2a-inner_class.diff
>
>
> In DERBY-2646, some improvements to org.apache.derby.iapi.util.UTF8Util were
> suggested:
> - remove unused private method isDerbyEOFMarker(), or possibly rewrite it
> to fit into skipInternal()
> - skipInternal() should return an instance of a private inner class instead
> of an array
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.