You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by GitBox <gi...@apache.org> on 2022/03/14 19:21:52 UTC

[GitHub] [daffodil] mbeckerle opened a new pull request #773: Cleanup solarcloud warning. Remove do-while(false) local block workar…

mbeckerle opened a new pull request #773:
URL: https://github.com/apache/daffodil/pull/773


   …ound
   
   The workaround is unnecessary. An IDE issue of some sort.
   
   DAFFODIL-2674


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



[GitHub] [daffodil] tuxji commented on a change in pull request #773: Cleanup solarcloud warning. Remove do-while(false) local block workar…

Posted by GitBox <gi...@apache.org>.
tuxji commented on a change in pull request #773:
URL: https://github.com/apache/daffodil/pull/773#discussion_r826459513



##########
File path: daffodil-tdml-lib/src/main/scala/org/apache/daffodil/tdml/TDMLRunner.scala
##########
@@ -694,15 +694,6 @@ abstract class TestCase(testCaseXML: NodeSeq, val parent: DFDLTestSuite) {
     roundTrip: RoundTrip,
     implString: Option[String]): Unit
 
-  // Provide ability to override existing (default) tunables
-  private def retrieveTunablesCombined(existingTunables: Map[String, String], cfg: DefinedConfig) = {
-    // Note, ++ on Maps replaces any key/value pair from the left with that on the
-    // right, so key/value pairs defined in tunables overrule those defiend in
-    // the config file
-    val combined = existingTunables ++ cfg.tunablesMap
-    combined
-  }
-

Review comment:
       Your PR & commit message doesn't mention this change - is it another cleanup (removing a function that's not used anymore)?




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



[GitHub] [daffodil] mbeckerle commented on a change in pull request #773: Cleanup solarcloud warning. Remove do-while(false) local block workar…

Posted by GitBox <gi...@apache.org>.
mbeckerle commented on a change in pull request #773:
URL: https://github.com/apache/daffodil/pull/773#discussion_r827217807



##########
File path: daffodil-tdml-lib/src/main/scala/org/apache/daffodil/tdml/TDMLRunner.scala
##########
@@ -694,15 +694,6 @@ abstract class TestCase(testCaseXML: NodeSeq, val parent: DFDLTestSuite) {
     roundTrip: RoundTrip,
     implString: Option[String]): Unit
 
-  // Provide ability to override existing (default) tunables
-  private def retrieveTunablesCombined(existingTunables: Map[String, String], cfg: DefinedConfig) = {
-    // Note, ++ on Maps replaces any key/value pair from the left with that on the
-    // right, so key/value pairs defined in tunables overrule those defiend in
-    // the config file
-    val combined = existingTunables ++ cfg.tunablesMap
-    combined
-  }
-

Review comment:
       Yes. Just deadcode that sonarcloud scanning found. 




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



[GitHub] [daffodil] mbeckerle merged pull request #773: Cleanup solarcloud warning. Remove do-while(false) local block workar…

Posted by GitBox <gi...@apache.org>.
mbeckerle merged pull request #773:
URL: https://github.com/apache/daffodil/pull/773


   


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



[GitHub] [daffodil] mbeckerle commented on a change in pull request #773: Cleanup solarcloud warning. Remove do-while(false) local block workar…

Posted by GitBox <gi...@apache.org>.
mbeckerle commented on a change in pull request #773:
URL: https://github.com/apache/daffodil/pull/773#discussion_r826344261



##########
File path: daffodil-cli/src/it/scala/org/apache/daffodil/tunables/TestCLITunables2.scala
##########
@@ -107,8 +106,8 @@ class TestCLITunables2 {
       } finally {
         shell.close()
       }
-    } while (false) // workaround scala local block bug.
-    do {

Review comment:
       Ok. 
   Interestingly, fixing this do-while thing..... failed to compile on the github automated CI. Compiles on my machine tho. 




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



[GitHub] [daffodil] stevedlawrence commented on a change in pull request #773: Cleanup solarcloud warning. Remove do-while(false) local block workar…

Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #773:
URL: https://github.com/apache/daffodil/pull/773#discussion_r826345373



##########
File path: daffodil-cli/src/it/scala/org/apache/daffodil/tunables/TestCLITunables2.scala
##########
@@ -107,8 +106,8 @@ class TestCLITunables2 {
       } finally {
         shell.close()
       }
-    } while (false) // workaround scala local block bug.
-    do {

Review comment:
       I was playing around with a small example, and for some reason I needed to provide the type on some variables. Might be the case here as well.




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



[GitHub] [daffodil] stevedlawrence commented on a change in pull request #773: Cleanup solarcloud warning. Remove do-while(false) local block workar…

Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #773:
URL: https://github.com/apache/daffodil/pull/773#discussion_r826303735



##########
File path: daffodil-cli/src/it/scala/org/apache/daffodil/tunables/TestCLITunables2.scala
##########
@@ -107,8 +106,8 @@ class TestCLITunables2 {
       } finally {
         shell.close()
       }
-    } while (false) // workaround scala local block bug.
-    do {

Review comment:
       Since you're making this change, can you also update to use `expectIn` instead of `expect` so we can remove the redirect? My concern is redirecting the input is going to cause stderr and stdout mixed up and might cause random failures when expecting strings depending on how things get mixed up. Might not actually be possible, but it's maybe good practice to avoid the redirect in favor of expectIn.




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