rmuir commented on PR #11923: URL: https://github.com/apache/lucene/pull/11923#issuecomment-1314486480
> just some question about cases like `merge.estimatedMergeBytes / 1024 / 1024.` changed to `merge.estimatedMergeBytes / 1024. / 1024.`: > Were they also found by this check or did you just change them because you have ssen them. Because the pattern here is different. Division with integers should not overflow, so I wonder why this was found by the errorprone check. > The new code is much better as it is consistent! yes, some of the time the code was doing `X / 1024 / 1024.` and other times it was doing `X / 1024. / 1024.` So it is the floating point portion of this NarrowCalculation check. It doesn't overflow when you do this, but it unnecessarily throws some precision away in the trash... so it is fixed to be consistent everywhere. I think for the cases we have it would not cause any bad bugs, but it was also easy enough to fix so that we can have the check enabled everywhere. -- 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
