tuxji commented on code in PR #919: URL: https://github.com/apache/daffodil/pull/919#discussion_r1081931393
########## xsrc/test/scripts/refactor/fix-gen.sh: ########## @@ -0,0 +1,28 @@ +cd $(dirname $0)/../../../.. Review Comment: Insert at the start of the file to make the output easier to understand: ``` #!/bin/bash -x ``` ########## xsrc/test/scripts/refactor/git-restore.sh: ########## @@ -0,0 +1,45 @@ +#!/bin/bash Review Comment: Change to `#!/bin/bash -x` to make the output easier to understand. ########## xsrc/test/scripts/refactor/refactor.sh: ########## @@ -0,0 +1,12 @@ + #!/bin/bash -e + +cd $(dirname $0)/../../../.. +REFACTOR_DIR=xsrc/test/scripts/refactor + +bash $REFACTOR_DIR/git-restore.sh +scala $REFACTOR_DIR/gen-symbol-table.scala daffodil-* > $REFACTOR_DIR/symbols.txt +bash $REFACTOR_DIR/rename-dirs.sh +bash $REFACTOR_DIR/fix-gen.sh Review Comment: Remove bash from each line so that the `#!/bin/bash -x` in each script will work as intended: ``` $REFACTOR_DIR/git-restore.sh scala $REFACTOR_DIR/gen-symbol-table.scala daffodil-* > $REFACTOR_DIR/symbols.txt $REFACTOR_DIR/rename-dirs.sh $REFACTOR_DIR/fix-gen.sh ``` Since you included a generated output file (symbols.txt) in this pull request, the above scala program overwrote symbols.txt and it turns out that the order of the symbols is not deterministic (my overwritten file had the same number of symbols but the order of the symbols was completely different). You should sort the symbols before outputting them or remove symbols.txt from the pull request to avoid unnecessary churn when you update the scripts in your next commit. ########## xsrc/test/scripts/refactor/refactor.sh: ########## @@ -0,0 +1,12 @@ + #!/bin/bash -e Review Comment: Remove the leading whitespace character and change to `#!/bin/bash -ex` to make the output easier to understand. ########## xsrc/test/scripts/refactor/rename-dirs.sh: ########## @@ -0,0 +1,106 @@ +#!/bin/bash Review Comment: Change to `#!/bin/bash -x` to make the output easier to understand. -- 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]
