You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by "stevedlawrence (via GitHub)" <gi...@apache.org> on 2023/02/21 20:23:37 UTC
[GitHub] [daffodil] stevedlawrence commented on a diff in pull request #968: Rename daffodil-runtime2
stevedlawrence commented on code in PR #968:
URL: https://github.com/apache/daffodil/pull/968#discussion_r1113509182
##########
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:
It would be nice if we could do something like this for the rest of Daffodil--some convention where it's understood everything with a with a certain name/directory is ALv2 licensed, but cannot have a licenese it it.
That said, I imagine this was the reason for the data/infoset split? I've found that I generally prefer all data and infoset files to be in the same directory. I don't feel strongly about it, but I've found the extra level of indirection usually just gets in the way and doesn't really add much.
##########
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:
I would prefer we don't include this change in this PR. If we want to reduce duplication, we should do for all of our CLI tests at the same PR in a consistent way. Changing only one test suite is what got us into the state we were in where every CLI test did something slightly different and it was a maintenance nightmare.
In general, I'm also not a huge fan of defining this path once globally. It's a lot of duplication, but it makes it much easier to debug tests when all the necessary input/output information is right in the test and we don't have to go searching for where variables are defined. It also gets confusing if we end up testing more schemas, which leads to `val schem1 = ...`, `val schma2 = ...`, which just gets confusing and unhelpful.
##########
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:
The osgi-refactor script has a Rat.scala file that references "runtime2". How do we want to handle that? I assume no changes and it's only there for historical reasons?
##########
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:
Was this put in the `cliTest` package for a reason, like some issue with OSGI?
##########
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:
In the `Documetation` section of this file there is a "Daffodil Runtime 2" link to confluence. We should rename that confluence page and update the link as part of this PR.
--
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