On Thu, 21 Mar 2024 22:23:44 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> Using Eclipse IDE to remove unused imports in **demo apps** (3D, Ensemble, >> etc.) and update the copyright year to 2024. Using wildcard for more than 10 >> static imports. >> >> >> -- >> >> This is a trivial change (though fairly large), 1 reviewer is probably >> enough. > > apps/samples/Ensemble8/src/app/java/ensemble/samplepage/Description.java line > 35: > >> 33: >> 34: import static ensemble.samplepage.SamplePage.INDENT; >> 35: import static ensemble.samplepage.SamplePageContent.title; > > I see that the static imports (for ensemble.xxx) were moved up to the top. Is > that deliberate? it's how the formatter is currently configured. do we have a preferred order? > apps/samples/Ensemble8/src/app/java/ensemble/samplepage/Description.java line > 55: > >> 53: import ensemble.SampleInfo; >> 54: import ensemble.SampleInfo.URL; >> 55: import ensemble.generated.Samples; > > I see that `ensemble.xxx` is moved after `javafx.xxx`; similarly, elsewhere I > see `com.sun.xxx` imports moved after. What's the algorithm your IDE uses to > sort imports? sort stuff that starts with "java" first and then everything > else alphabetically? this is how the IDE formatter is currently configured - it should not matter, especially since it dos not use wildcards (except for static imports). > apps/samples/Ensemble8/src/samples/java/ensemble/samples/graphics2d/puzzle/Piece.java > line 48: > >> 46: /** >> 47: * Node that represents a puzzle piece >> 48: */ > > Hmm. This is more than just fixing imports. Were these lines reformatted > because they were touching the modified import lines? Anything other than > changes to the import statements will require extra time / mental energy to > review. this is just one spot that bothered me. reformatted since I have to delete a blank line. won't happen again, I promise. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1420#discussion_r1534768134 PR Review Comment: https://git.openjdk.org/jfx/pull/1420#discussion_r1534769787 PR Review Comment: https://git.openjdk.org/jfx/pull/1420#discussion_r1534771453
