[GitHub] [incubator-daffodil] mbeckerle commented on pull request #481: ADD test for "%%" to bypass existing test for "%NL"

[GitBox <gi...@apache.org>]

mbeckerle commented on pull request #481:
URL: https://github.com/apache/incubator-daffodil/pull/481#issuecomment-771714932


   If the code is self explanatory, because the lists of allowed/disallowed entities are explicitly sitting there, then yeah, no comments are needed. Given a list of them though, it may not be obvious which one is not there. So a comment like // Not ES
   may be helpful.
   Alternatively you can do something like:
   ```
   val entitiesWithoutES = Seq("%NL;", ....)
   ```
   To make it self-documenting without using comments.
   
   But in general, push your changes and let's see how it is coming out. 
   I often push changes and immediately put some comments on the PR myself to ask people what they think about this or that aspect. Just like your question here. 


----------------------------------------------------------------
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.

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