[ https://issues.apache.org/jira/browse/MCOMPILER-381?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17789630#comment-17789630 ]
ASF GitHub Bot commented on MCOMPILER-381: ------------------------------------------ jorsol commented on code in PR #181: URL: https://github.com/apache/maven-compiler-plugin/pull/181#discussion_r1404815615 ########## src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java: ########## @@ -1539,36 +1546,41 @@ private static List<String> removeEmptyCompileSourceRoots(List<String> compileSo * generated classes and if we got a file which is >= the build-started timestamp, then we caught a file which * got changed during this build. * - * @return <code>true</code> if at least one single dependency has changed. + * @return {@code true} if at least one single dependency has changed. */ - protected boolean isDependencyChanged() { - if (session == null) { + private boolean isDependencyChanged() { Review Comment: Same, this should have never been protected, is an internal method. A good practice is to always start with the most restricted modifier (private) and then if needed open it to package-protected, then protected and finally public. Again, this should not break anything since this class is not intended to be subclassed externally (unless I'm missing a weird use case and maven plugins can be used as public APIs). > Refactoring needed for isDependencyChanged / Using fileExtensions > (AbstractCompilerMojo) > ---------------------------------------------------------------------------------------- > > Key: MCOMPILER-381 > URL: https://issues.apache.org/jira/browse/MCOMPILER-381 > Project: Maven Compiler Plugin > Issue Type: Improvement > Affects Versions: 3.8.1 > Reporter: Karl Heinz Marbaise > Priority: Minor > Fix For: 3.12.0 > > > The code in the class AbstractCompilerMojo has a method > {{isDependencyChanged}} which uses the attribute {{fileExtensions}} which is > being changed within the {{isDependencyChanged}} method. This attribute is > also being used by the method {{hasNewFile}} which is a kind of confusing (a > control via a global variable). > Furthermore a change in {{isDependencyChanged}} where replacing {{".class"}} > with {{"class"}} results in a [fail which is described here|MCOMPILER-379]. > It should be investigated how this code can be made more clear and maybe > easier to understand. -- This message was sent by Atlassian Jira (v8.20.10#820010)