Hi Piotr, Thanks for the details. The good new is that it was caught fairly quickly. Looking forward to reviewing the RC...
Gary On Sat, Nov 16, 2024, 12:06 PM Piotr P. Karwasz <pi...@mailing.copernik.eu> wrote: > Hi Gary, > > On 16.11.2024 17:41, Gary Gregory wrote: > > [1]https://github.com/apache/logging-log4j2/issues/3143 > > > > This issue points to the merged PR > > https://github.com/apache/logging-log4j2/pull/2936, a large change set > with > > zero tests. This isn't great since there are no tests to catch > regressions > > :-( > > When PR #2936 was reviewed, bug #3143 wasn't known. > > While PR #2936 (for the `2.25.x` release) is not affected by the bug, > the bug itself was caused by a backport of PR #2936 to the `2.24.x` > branch (see PR #2961[1]). The bug itself is due to the reachability > rules for Java local variables: a variable becomes unreachable **not** > at the end of the syntactic scope, but right after its last > **non-trivial** usage (see [2] for more details). That is not an easy > problem to catch in a review IMHO and certainly not something you can > easily test. > > [1] https://github.com/apache/logging-log4j2/pull/2961 > > [2] https://shipilev.net/jvm/anatomy-quarks/8-local-var-reachability/ > > > [2]https://github.com/apache/logging-log4j2/pull/3209 > > > > LGTM: If I apply the patch, the test is green, then when I revert the > > 'main' changes, the test fails. Nice work :-) > > > > [3]https://github.com/apache/logging-log4j2/pull/3210 > > > > LGTM: If I apply the patch, the 2 tests are green, then when I revert the > > 'main' changes, the 2 tests fail. Nice work :-) > > > > So it all looks good :-) > > Could you formally approve those PRs? Thanks. > > Piotr >
