On Thu, 6 Mar 2025 21:49:17 GMT, Sean Mullan <mul...@openjdk.org> wrote:
>> Artur Barashev has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains 15 additional >> commits since the last revision: >> >> - Merge branch 'master' into JDK-8349583 >> - Remove the fix for JDK-8350807. Update documentation. >> - Update documentation and unit tests to signal TLS scope case-insensitivity >> - Update 2 more copyrights >> - Update TLS version in one more unit test >> - - Check signature schemes that are enabled specifically for the handshake >> when HANDSHAKE_SCOPE is specified >> - Update copyright >> - Update HTTPS tests that are broken because we also fix JDK-8350807 on >> the server side as a side-effect >> - Revert "Restore original arguments for getSupportedAlgorithms() calls" >> >> This reverts commit 4b335619ee6a79a6f609fe98c5339588a6a1342a. >> - Restore original arguments for getSupportedAlgorithms() calls >> - - Refactor code to use existing "usage" constraint. >> - Rename SSLCryptoScope to SSLScope, make it public. >> - Merge branch 'master' into JDK-8349583 >> - ... and 5 more: https://git.openjdk.org/jdk/compare/3466582c...7a786e0d > > src/java.base/share/classes/sun/security/ssl/SSLScope.java line 28: > >> 26: /* >> 27: * Scopes defining different parts of TLS protocol. >> 28: */ > > Can you move the class description below the package? Done. > src/java.base/share/classes/sun/security/ssl/SSLScope.java line 39: > >> 37: >> 38: // Note: the SSLScope is case-insensitive. >> 39: public static SSLScope nameOf(String name) { > > `valueOf` is probably a better name, and used by other Enum classes for this > pattern. Actually I couldn't find a single example of using `valueOf` with String argument in `sun/security/ssl`, while there are plenty of example of using `nameOf`. So I just followed the established convention: src/java.base/share/classes/sun/security/ssl/SSLScope.java:39: src/java.base/share/classes/sun/security/ssl/ProtocolVersion.java:214: src/java.base/share/classes/sun/security/ssl/CipherSuite.java:925: src/java.base/share/classes/sun/security/ssl/NamedGroup.java:352: src/java.base/share/classes/sun/security/ssl/SignatureScheme.java:349: ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/23681#discussion_r1985293588 PR Review Comment: https://git.openjdk.org/jdk/pull/23681#discussion_r1985290573