desruisseaux commented on code in PR #1027:
URL:
https://github.com/apache/maven-compiler-plugin/pull/1027#discussion_r2756349814
##########
src/main/java/org/apache/maven/plugin/compiler/ToolExecutor.java:
##########
@@ -72,11 +73,6 @@
* @author Martin Desruisseaux
*/
public class ToolExecutor {
- /**
- * The locale for diagnostics, or {@code null} for the platform default.
- */
- private static final Locale LOCALE = null;
-
Review Comment:
> then it will be a bug so we must not do it, it can be acceptable to make
it not static but it would stay dead code
If someone puts `Locale.JAPANESE`, some texts will appear in Japanese, which
is the expected behaviour, not a bug. It is not dead code neither, it is the
declaration of a constant (for now), and `null` is a perfectly valid constant
value.
Configuration may come in the future, which is precisely the reason why this
constant is defined: if some day we want to support `Locale` configuration, we
will just need to remove the `static final` part. The purpose of this constant
is to save time in the future by avoiding the need to search where a locale is
used. With the change in this pull request, `LOCALE` is replaced by `null` in
method call arguments. It is not easy to guess by reading the code that the
argument was a locale. With the `LOCALE` constant, it is obvious.
Please revert. It is not because the value of a constant is zero or null
that it is dead code.
--
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]
