On Tue, 29 Nov 2022 16:46:43 GMT, Per Minborg <pminb...@openjdk.org> wrote:
> This PR proposes a variety of modernisations to the `jdk.sctp` module. > > During the fix of https://bugs.openjdk.org/browse/JDK-8296024, several > improvement areas were identified including: > > * Replacing duplicate code segments > * Making certain fields final > * Using enhanced switch > * Using records > * Fixing typos > * Marking fields participating in serialisation with `@Serial` > * Modernizing toString() implementations > * Using pattern matching > * Using diamond operators It is a bit unusual to use a local class as a holder class (the typical manner is to a have a private static final nested class) but we didn't have local classes until recently - so maybe that's OK. I assume there's only one version of that class and it is static? src/jdk.sctp/share/classes/sun/nio/ch/sctp/SctpStdSocketOption.java line 32: > 30: public record SctpStdSocketOption<T>(String name, > 31: Class<T> type, > 32: int constValue) This changes the implementation of `toString`, `equals` and `hashCode`. Have you verified that it is OK to do so? src/jdk.sctp/unix/classes/sun/nio/ch/sctp/SctpMultiChannelImpl.java line 33: > 31: import java.io.FileDescriptor; > 32: import java.io.IOException; > 33: import java.util.*; It would be better to avoid wildcards in regular imports. ------------- PR: https://git.openjdk.org/jdk/pull/11418