[GitHub] [daffodil] tuxji commented on a diff in pull request #968: Rename daffodil-runtime2

["tuxji (via GitHub)" <gi...@apache.org>]

tuxji commented on code in PR #968:
URL: https://github.com/apache/daffodil/pull/968#discussion_r1113584070


##########
daffodil-cli/src/test/scala/org/apache/daffodil/cli/cliTest/TestCLIGenerateC.scala:
##########
@@ -31,22 +32,18 @@ import org.junit.Test
  */
 class TestCLIGenerateC {
 
-  @Test def test_CLI_Generate_schema(): Unit = {
-    val schema = path(
-      "daffodil-runtime2/src/test/resources/org/apache/daffodil/runtime2/ex_nums.dfdl.xsd",
-    )
+  val schema: Path = path(
+    "daffodil-codegen-c/src/test/resources/org/apache/daffodil/codegen/c/ex_nums.dfdl.xsd",
+  )

Review Comment:
   Sure, I'll remove the schema change from the PR.



##########
daffodil-cli/src/test/scala/org/apache/daffodil/cli/cliTest/TestBlob.scala:
##########
@@ -15,7 +15,7 @@
  * limitations under the License.
  */
 
-package org.apache.daffodil.cliTest
+package org.apache.daffodil.cli.cliTest

Review Comment:
   The tests were always in package `org.apache.daffodil.cliTest` even before the OSGi refactoring was done.  The refactoring scripts added a `cli` directory and moved all the files down one level below the `cli` directory, but they didn't touch these tests' package lines.
   
   OSGi loaders are fine as long as everything in daffodil-cli starts with the same package (org.apache.daffodil.cli) and nothing else anywhere else starts with the same package.  Whatever names we choose after `cli` is up to us and doubly so for tests, which are rarely packaged into jars, making them invisible to OSGi loaders anyway.  If we want to, we can remove the `cliTest` directory and move the tests up one level into the `cli` directory instead.



##########
project/Rat.scala:
##########
@@ -112,58 +112,7 @@ object Rat {
     file("daffodil-tdml-lib/src/test/resources/test/tdml/test.txt"),
     file("daffodil-tdml-processor/src/test/resources/test/tdml/test.bin"),
     file("daffodil-tdml-processor/src/test/resources/test/tdml/test.txt"),
-    file(
-      "daffodil-test/src/test/resources/org/apache/daffodil/runtime2/ISRM_green_to_orange_60000.0.dat",
-    ),
-    file(
-      "daffodil-test/src/test/resources/org/apache/daffodil/runtime2/ISRM_green_to_orange_60000.1.dat",
-    ),
-    file(
-      "daffodil-test/src/test/resources/org/apache/daffodil/runtime2/ISRM_orange_to_green_60002.dat",
-    ),
-    file(
-      "daffodil-test/src/test/resources/org/apache/daffodil/runtime2/MPU_green_to_orange_60004.dat",
-    ),
-    file(
-      "daffodil-test/src/test/resources/org/apache/daffodil/runtime2/MPU_orange_to_green_60006.0.dat",
-    ),
-    file(
-      "daffodil-test/src/test/resources/org/apache/daffodil/runtime2/MPU_orange_to_green_60006.1.dat",
-    ),
-    file("daffodil-test/src/test/resources/org/apache/daffodil/runtime2/collisions.dat"),
-    file("daffodil-test/src/test/resources/org/apache/daffodil/runtime2/egress_xdcc_bw.11.dat"),
-    file("daffodil-test/src/test/resources/org/apache/daffodil/runtime2/egress_xdcc_bw.12.dat"),
-    file("daffodil-test/src/test/resources/org/apache/daffodil/runtime2/egress_xdcc_bw.13.dat"),
-    file("daffodil-test/src/test/resources/org/apache/daffodil/runtime2/egress_xdcc_bw.14.dat"),
-    file("daffodil-test/src/test/resources/org/apache/daffodil/runtime2/egress_xdcc_bw.15.dat"),
-    file("daffodil-test/src/test/resources/org/apache/daffodil/runtime2/egress_xdcc_bw.16.dat"),
-    file("daffodil-test/src/test/resources/org/apache/daffodil/runtime2/egress_xdcc_bw.17.dat"),
-    file("daffodil-test/src/test/resources/org/apache/daffodil/runtime2/egress_xdcc_bw.18.dat"),
-    file("daffodil-test/src/test/resources/org/apache/daffodil/runtime2/egress_xdcc_bw.19.dat"),
-    file("daffodil-test/src/test/resources/org/apache/daffodil/runtime2/egress_xdcc_bw.20.dat"),
-    file(
-      "daffodil-test/src/test/resources/org/apache/daffodil/runtime2/ingress_xdcc_bw.111.dat",
-    ),
-    file(
-      "daffodil-test/src/test/resources/org/apache/daffodil/runtime2/ingress_xdcc_bw.112.dat",
-    ),
-    file(
-      "daffodil-test/src/test/resources/org/apache/daffodil/runtime2/ingress_xdcc_bw.113.dat",
-    ),
-    file(
-      "daffodil-test/src/test/resources/org/apache/daffodil/runtime2/ingress_xdcc_bw.114.dat",
-    ),
-    file(
-      "daffodil-test/src/test/resources/org/apache/daffodil/runtime2/ingress_xdcc_bw.115.dat",
-    ),
-    file(
-      "daffodil-test/src/test/resources/org/apache/daffodil/runtime2/ingress_xdcc_bw.116.dat",
-    ),
-    file("daffodil-test/src/test/resources/org/apache/daffodil/runtime2/orion.aptina.dat"),
-    file("daffodil-test/src/test/resources/org/apache/daffodil/runtime2/orion.camera.dat"),
-    file("daffodil-test/src/test/resources/org/apache/daffodil/runtime2/orion.command.dat"),
-    file("daffodil-test/src/test/resources/org/apache/daffodil/runtime2/orion.limits.dat"),
-    file("daffodil-test/src/test/resources/org/apache/daffodil/runtime2/orion.video.dat"),
+    file("daffodil-test/src/test/resources/org/apache/daffodil/codegen/c/data"),

Review Comment:
   I agree that there are a small number of files, it's easier to keep everything in the same directory.  When the number of files grows larger, then grouping them into different directories allows us to reduce Rat.scala's size.  Another advantage I've noticed is that you can run daffodil parse/unparse and c/daffodil parse/unparse commands on data and infosets without clobbering existing files, which allows you to compare output files to existing files more easily as well.  You just have to make sure your parse/unparse commands consistently write their output files to directories where the output files aren't present already.



##########
DEVELOP.md:
##########
@@ -224,6 +224,7 @@ daffodil/
 │   ├── bin.NOTICE              - Contains notices of Daffodil and subcomponents
 │   ├── build.sbt               - Tells sbt how to build CLI and installers
 │   └── src/                    - Contains CLI source code, tests, scripts, etc.
+├── daffodil-codegen-c/         - Contains Daffodil's C code generator

Review Comment:
   Good catch, I'd grepped for runtime2 but not "runtime 2" with any separating character.  That place's the only place and I'll fix it.  



##########
project/Rat.scala:
##########
@@ -40,7 +40,7 @@ object Rat {
     file("daffodil-cli/src/windows/dialog.bmp"),
 
     // generated_code.[ch] examples
-    file("daffodil-runtime2/src/test/c/examples"),
+    file("daffodil-codegen-c/src/test/c/examples"),

Review Comment:
   I know, I saw the runtime2 still mentioned in the scripts too.  I left the scripts alone because they're historical artifacts that @mbeckerle wanted to keep around in case someone needs to develop new refactoring scripts again.  It would've been easy enough to get them out of git history, but only for someone who remembers that the scripts were there.  I have no objections changing runtime2 in them just to have a clean slate, however.



-- 
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: commits-unsubscribe@daffodil.apache.org

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