Got so excited that I forgot to say which of the two options exactly I meant - running the analysis only on changed files after the initial full pass is done sounds like a good improvement to me
On Fri, 16 Jun 2023 at 7:43, Ekaterina Dimitrova <e.dimitr...@gmail.com> wrote: > I think this is a great idea and it will probably reduce the time to run > it. Thank you! > > On Fri, 16 Jun 2023 at 7:40, Jacek Lewandowski < > lewandowski.ja...@gmail.com> wrote: > >> Additional question is whether we want to run the checks against the >> whole project or just against the file changes between the feature branch >> and the target release branch? >> >> >> - - -- --- ----- -------- ------------- >> Jacek Lewandowski >> >> >> pt., 16 cze 2023 o 13:09 Aleksey Yeshchenko <alek...@apple.com> >> napisał(a): >> >>> Sounds like a clear improvement to me. Only once this check flagged a >>> legitimate issue I missed, if I’m remembering correctly. All other >>> instances have just been annoyances, forcing to add a redundant suppressed >>> annotation. >>> >>> On 15 Jun 2023, at 19:01, Ekaterina Dimitrova <e.dimitr...@gmail.com> >>> wrote: >>> >>> Hi everyone, >>> Happy Thursday! >>> Some time ago, Jacek raised the point that ant eclipse-warnings is >>> generating too many false positives and not really working as expected. >>> (CASSANDRA-18239) >>> >>> Reminder: ant eclipse-warnings is a task we run with the goal to check >>> Cassandra code - static analysis to warn on unsafe use of Autocloseable >>> instances; checks against two related particular compiler options >>> >>> While trying to upgrade ECJ compiler that we use for this task >>> (CASSANDRA-18190) so we can switch the task from running it with JDK8 to >>> JDK11 in preparation for dropping JDK8, I hit the following issues: >>> - the latest version of ECJ is throwing more than 300 Potential Resource >>> Leak warnings. I looked at 10-15, and they were all false positives. >>> - Even if we file a bug report to the Eclipse community, JDK11 is about to >>> be removed with the next version of the compiler >>> >>> So I shared this information with Jacek. He came up with a different >>> solution: >>> It seems we already pull through Guava CheckerFramework with an MIT >>> license, which appears to be acceptable according to this link - >>> https://www.apache.org/legal/resolved.html#category-a >>> He already has an initial integration with Cassandra which shows the >>> following: >>> - CheckerFramework does not understand the @SuppressWarnings("resource") >>> (there is a different one to be used), so it is immediately visible how it >>> does not report all those false positives that eclipse-warnings does. On >>> the flip side, I got the feedback that what it has witnessed so far is >>> something we should investigate. >>> - Also, there are additional annotations like @Owning that let you fix many >>> problems at once because the tool understands that the ownership of the >>> resources was passed to another entity; It also enables you to do something >>> impossible with eclipse-warnings - you can tell the tool that there is >>> another method that needs to be called to release the resources, like >>> release, free, disconnect, etc. >>> - the tool works with JDK8, JDK11, JDK17, and JDK20, so we can backport it >>> even to older branches (while at the same time keeping eclipse-warnings >>> there) >>> - though it runs 8 minutes so, we should not run it with every test, some >>> reorganization around ant tasks will be covered as even for >>> eclipse-warnings it was weird to call it on every single test run locally >>> by default >>> >>> >>> If there are no concerns, we will continue replacing ant eclipse-warnings >>> with the CheckerFramework as part of CASSANDRA-18239 and CASSANDRA-18190 in >>> trunk. >>> >>> Best regards, >>> >>> Ekaterina >>> >>> >>>
