On Thu, 15 Feb 2024 12:19:31 GMT, Magnus Ihse Bursie <i...@openjdk.org> wrote:
> Since jcheck only checks file in a commit, there is a possibility of us > getting files in the repository that would not be accepted by jcheck. This > can happen when extending the set of files checked by jcheck, or if jcheck > changes how it checks files (perhaps due to bug fixes). > > I have now run jcheck over the entire code base, and fixed a handful of > issues. With these changes, jcheck accept the entire code base. Some general remarks: The problems in .m4 files where not properly detected and fixed when I added .m4 to the list of checked files. The properties files were recently checked by me, so these suprrised me. It turned out I had misunderstood the jcheck criteria: I thought only leading tabs were disallowed, but it is actually tabs anywhere in the file that are banned. In general, I have replaced tab characters with spaces aligning to tab stops at 8 characters distance. I'll leave some remarks for individual files. src/java.naming/share/classes/com/sun/jndi/ldap/jndiprovider.properties line 10: > 8: > java.naming.factory.object=com.sun.jndi.ldap.obj.AttrsToCorba:com.sun.jndi.ldap.obj.MarshalledToObject > 9: > 10: # RemoteToAttrs: Turn RMI/JRMP and RMI/IIOP object into > javaMarshalledObject or I adjusted this tab stop (with one space) so it aligned properly with the line above; I think that was the intention. src/jdk.jdeps/share/classes/com/sun/tools/jdeps/resources/jdkinternals.properties line 41: > 39: jdk.internal.ref.Cleaner=Use java.lang.ref.PhantomReference @since 1.2 or > java.lang.ref.Cleaner @since 9 > 40: sun.awt.CausedFocusEvent=Use java.awt.event.FocusEvent::getCause @since 9 > 41: sun.font.FontUtilities=See java.awt.Font.textRequiresLayout @since 9 This tab just looked out of place, so I replaced it with a space. (Rhyming not intentional...) test/hotspot/jtreg/containers/docker/JfrReporter.java line 52: > 50: } > 51: } > 52: } I'm not sure how a Java file ever got into the code base with trailing spaces, but a guess is that there have been a bug in jcheck that did not properly check for trailing whitespace at the end of the file, or something like that. test/jdk/java/util/Currency/currency.properties line 18: > 16: SB=EUR,111,2, 2099-01-01T00:00:00 > 17: US=euR,978,2,2001-01-01T00:00:00 > 18: ZZ = ZZZ , 999 , 3 This looks weird, but so did the original code. I assume this is to clearly indicate this as a end of list marker. ------------- PR Comment: https://git.openjdk.org/jdk/pull/17871#issuecomment-1945992837 PR Review Comment: https://git.openjdk.org/jdk/pull/17871#discussion_r1490931378 PR Review Comment: https://git.openjdk.org/jdk/pull/17871#discussion_r1490931979 PR Review Comment: https://git.openjdk.org/jdk/pull/17871#discussion_r1490933432 PR Review Comment: https://git.openjdk.org/jdk/pull/17871#discussion_r1490934063
