You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2022/09/23 19:12:08 UTC

[GitHub] [commons-text] garydgregory commented on pull request #312: TEXT-216: Add HTML5 Entities

garydgregory commented on PR #312:
URL: https://github.com/apache/commons-text/pull/312#issuecomment-1256580580

   > @rbunel35, thank you for the PR and apologies for the late reply.
   > 
   > This looks like useful functionality to me, although I'm slightly concerned about the new size of `EntityArrays`. (If the character maps were accessed by static methods instead of constants, I would recommend using separate private classes to initialize each entity grouping.) Regardless, could you add a unit test that iterates through each HTML5 entity in the official reference and ensures that they are all accounted for and escaped/unescaped correctly? One way to do this would be to convert the JSON reference document into a properties file and load the properties file during the test.
   
   I agree about splitting out the large new arrays into a new class. 
   
   In general, new public and protected elements need to be documented with the Javadoc since tag. Also, we should make as little as possible new or protected to give us maximum flexibility for maintenance.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org