Remove extra variable #2398
Remove extra variable #2398
Conversation
This reverts commit 32d9512.
|
|
||
| String substValue = processSubstitution( | ||
| variable, org, cycleMap, configProps, callback, postprocessor, defaultsToEmptyString); |
There was a problem hiding this comment.
yes, should be covered by https://pmd.github.io/pmd/pmd_rules_java_bestpractices.html#unusedassignment
There was a problem hiding this comment.
seems its currently renaming not removal.
| @@ -283,7 +283,7 @@ private static String doSubstVars( | |||
| } | |||
|
|
|||
| private static String processSubstitution( | |||
| String variable, | |||
| final String variable, | |||
There was a problem hiding this comment.
boilerplate tradeoff naturally evolves to https://pmd.github.io/pmd/pmd_rules_java_bestpractices.html#avoidreassigningparameters.
There was a problem hiding this comment.
Agreed, I don't like final parameters.
There was a problem hiding this comment.
assuming you mean the explicit final declaration. Params should be treated final by default to avoid coupling, thus bugs.
|
Test failures might be related but unclear why it only fails on java 21 |
| @@ -284,8 +283,7 @@ private static String doSubstVars( | |||
| } | |||
|
|
|||
| private static String processSubstitution( | |||
| String variable, | |||
| String org, | |||
| final String variable, | |||
There was a problem hiding this comment.
| final String variable, | |
| String variable, |
pr title? it seems more re |
|
No, it's definitely removed |
|
yes sorry from 7 to 6 params. |
org is unclear and unneeded. Probably meant "original"? In any case, we don't need it.