uschindler commented on PR #15047: URL: https://github.com/apache/lucene/pull/15047#issuecomment-3209373104
I approved this PR already. The problem with the 10.x branch and changes in MemorySegment code are that we don't run ecj linter checks there (in the MRJAR, as ECJ not easily supports those apijar stub JARs). But when I opened the 10.x branch in Eclipse on my local machine I was flooded with those "unused" warnings caused by your fix. Therefore I asked to revert. Actually, the code like it was before (with the usused variable) was also not "bad" from beginning as it was consistent! The MMap code relies on that "exception handler logic" and has always same pattern in exception handling. For better readbility, I refactored the "catch" block to be separate methods. All those methods take the thrown exception as first parameter. Just the one you fixed here did not use the parameter (but it could at a later version of the code). The other catch handler method handling NullPointerException and other RuntimeExceptions uses the exception parameter to make decissions if the index input was closed or if it is some "real" problem. So from the code consistency the code in 10.x is more consistent now than the code in main. But I am fine with the main change, just the backport broke warnings. -- 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: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
