stevedlawrence commented on code in PR #799:
URL: https://github.com/apache/daffodil/pull/799#discussion_r886738676


##########
daffodil-tdml-lib/src/test/scala/org/apache/daffodil/tdml/UnitTestTDMLRunner.scala:
##########
@@ -345,6 +345,46 @@ class UnitTestTDMLRunner {
     assertEquals(expected, actual)
   }
 
+  @Test def testCommentBit(): Unit = {
+        val xml = <document bitOrder="LSBFirst">
+                <documentPart type="bits">00000010 //this is a 
label111</documentPart>

Review Comment:
   I'm not sure I'm sold on the the clutter argument--it's not great, but it's 
not awful.
   
   That said, the bug mentions CDATA, which makes me think of what I think is a 
strong argument against using XML comments, though isn't mentioned in the bug: 
XML comments aren't treated like comments in CDATA blocks, it's just normal 
text.
   
   It's not uncommon for TDML tests to used CDATA blocks in the documentPart to 
ensure IDE's don't rewrap XML and mess up formatting. My main reason for using 
XML comments is you could just do something like `(testSuite \ "document" \ 
"documentPart").text` and all the XML comments would be automatically removed. 
But that's not the case with XML Comments in CDATA, so we would still need 
special comment removing code anyways. And if we're doing that, we might as 
well pick something without any potential confusion that comes with XML 
comments in CDATA, and is maybe a little prettier.
   
   I agree that adding /* */ feels important. Inline comments seem like they 
could be used a lot.
   
   



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to