On Sat, 30 Nov 2024 18:39:01 GMT, Eirik Bjørsnøs <eir...@openjdk.org> wrote:
>> Please review this PR which applies various cleanups to >> `sun.net.www.protocol.file.FileURLConnection`. >> >> This class is known to be an _old, intricate, and hard to maintain piece of >> code._ However, there are some relatively straightforward and safe cleanups >> possible which improve readability and makes it easier to reason about >> what's going on in this class. >> >> In this PR, I have chosen to make each individual small change a separate >> commit. This to assist review of each individual change, which can otherwise >> disappear a bit when reviewing the PR as a whole. >> >> A detailed listing of each commit follows in a separate comment. >> >> This is a pure cleanup PR, no tests are added or updated in this PR. > > Eirik Bjørsnøs has updated the pull request incrementally with one additional > commit since the last revision: > > Replace String::valueOf with Long::toString I think these clean up changes are fine. There are a few places in this PR which changes the `File.toString()` usage to `File.getPath()`. `java.io.File` can be subclassed and the `toString()` can be overridden, so in theory this could result in different behaviour. However, the `sun.net.www.protocol.file.FileURLConnection` is an internal class and its package isn't exported. Looking at where we instantiate this class, we always pass a `java.io.File` instance. So I believe this change to use `File.getPath()` is fine and the right thing to do. Please wait for another review before integrating. In the meantime, I'll run our internal CI tests for this change. ------------- Marked as reviewed by jpai (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/22459#pullrequestreview-2477547502
