On Sat, 5 Apr 2025 19:36:33 GMT, Andrey Turbanov <aturba...@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.
>
> src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsFileChooserUI.java
>  line 1061:
> 
>> 1059:     //
>> 1060:     @SuppressWarnings("serial") // Superclass is not serializable 
>> across versions
>> 1061:     final class DirectoryComboBoxRenderer extends 
>> DefaultListCellRenderer  {
> 
> Suggestion:
> 
>     final class DirectoryComboBoxRenderer extends DefaultListCellRenderer {

There are many places where such a change could be applied, but I intentionally 
skipped them so that reviewers can be confident there are no other 
modifications beyond adding final and @Override.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/24170#discussion_r2030306330

Reply via email to