On Sat, 22 Mar 2025 00:49:25 GMT, Sergey Bylokhov <s...@openjdk.org> wrote:
> The `java.desktop` module currently lacks proper use of the `@Override` > annotation for methods and the `final` modifier for classes. While similar > changes were previously made in the > [JavaSound](https://github.com/openjdk/jdk/commit/e0c7d59246cf36644d494eced76e4b9d96ff1ded#diff-ae3e5f9c40fe25ef03e7a89844de174ef5c15e6179d769e2a4bcb7e73688c9b5) > (and some of the classes on demand), these changes are not as critical now > due to the new jdk "encapsulation", but they are still useful for improving > code consistency. > > To make the code review process easier I have made the following changes: > > 1. I chose `java.desktop/windows` as the starting package because it contains > a relatively small number of classes > 2. The public API was not affected so there is no need to worry about a CSR > 3. In a few places I removed the `final` modifier from methods since they > were already defined in final classes > > Note: I will submit additional patches in this area later because: > > 1. Only lines with `@Override` and `final` were modified to keep the diff > clear - header dates were not updated (that could be covered by one patch > similar to [this](https://bugs.openjdk.org/browse/JDK-8345797)) > 2. I skipped adding `@Override` for anonymous classes > > Any feedback or suggestions are welcome! > > Here is a > [link](https://patch-diff.githubusercontent.com/raw/openjdk/jdk/pull/24170.diff) > to a simple diff file, it might be more convenient for reviewing the changes. > > To download the diff file and filter only the modified lines you can use the > following script: > > > curl -s > https://patch-diff.githubusercontent.com/raw/openjdk/jdk/pull/24170.diff -o > d.txt && grep -E '^+|^-' d.txt > > > The build was successful, and I ran all the open jtreg tests without issues. > But it's possible that some closed tests may be affected by these changes. It > would be good to verify this by mach5 to ensure everything works as expected. The github UI is un-usable with this many files. So commenting here. The pattern below seems odd. What is the point of the protected modifier on a final class ? On a final method of a protected class, that I can see .. but this looks odd. - protected class FilterComboBoxModel extends AbstractListModel<FileFilter> implements ComboBoxModel<FileFilter>, + protected final class FilterComboBoxModel extends AbstractListModel<FileFilter> implements ComboBoxModel<FileFilter>, - protected class DirectoryComboBoxModel extends AbstractListModel<File> implements ComboBoxModel<File> { + protected final class DirectoryComboBoxModel extends AbstractListModel<File> implements ComboBoxModel<File> { ------------- PR Comment: https://git.openjdk.org/jdk/pull/24170#issuecomment-2776795601
