Re: RFR: 8310890: Normalize identifier names
On Mon, 26 Jun 2023 18:44:42 GMT, Pavel Rappo wrote:
>> make/data/charsetmapping/charsets line 149:
>>
>>> 147: package sun.nio.cs
>>> 148: typesbcs
>>> 149: histname ISO8859_2
>>
>> Should this column be re-aligned with the longer name?
>
> I thought about it before publishing the PR. I decided not to re-align
> anything because (i) the change would be bigger and (ii) the fact that there
> was already a property that is similarly misaligned; search for:
>
> internal true
If you are concerned with functionality rather than looks, then I can tell you
this:
1. The build succeeds and tier1 tests pass.
2. The code that parses that file expect one or more whitespace characters as a
separator:
String[] tokens = line.split("\\s+");
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14653#discussion_r1242629052
Re: RFR: 8310890: Normalize identifier names
On Mon, 26 Jun 2023 18:31:06 GMT, Jens Lidestrom wrote: >> Please review this cleanup PR to normalize names of identifiers which are >> Java variables/fields or tokens in text files. Those names either contain a >> pronoun that is very rarely used in code, or seem like they contain such a >> pronoun, which, in fact, they don't. Either way, the goal is to improve >> readability and clarity. >> >> Also, this PR fixes a few related typos. > > make/data/charsetmapping/charsets line 149: > >> 147: package sun.nio.cs >> 148: typesbcs >> 149: histname ISO8859_2 > > Should this column be re-aligned with the longer name? I thought about it before publishing the PR. I decided not to re-align anything because (i) the change would be bigger and (ii) the fact that there was already a property that is similarly misaligned; search for: internal true - PR Review Comment: https://git.openjdk.org/jdk/pull/14653#discussion_r1242617579
Re: RFR: 8310890: Normalize identifier names
On Mon, 26 Jun 2023 18:21:07 GMT, Roger Riggs wrote: >> Please review this cleanup PR to normalize names of identifiers which are >> Java variables/fields or tokens in text files. Those names either contain a >> pronoun that is very rarely used in code, or seem like they contain such a >> pronoun, which, in fact, they don't. Either way, the goal is to improve >> readability and clarity. >> >> Also, this PR fixes a few related typos. > > src/java.base/share/classes/java/util/EnumMap.java line 690: > >> 688: Object otherValue = em.vals[i]; >> 689: if (otherValue != ourValue && >> 690: (otherValue == null || !otherValue.equals(ourValue))) > > Is this the same as java.util.Objects: > `!Objects.equals(vals[i], em.vals[i]);` You are right: we can fold this if-else construct into that utility method. That said, it in *this* PR, I'd prefer not to. The reason is that I've been preparing a much bigger PR that uses Objects.equals as well as other utility methods and modern language features to improve some equals, hashCode, and compareTo implementations across java.base. I'm planning to publish that PR soon. In fact, this change to Java variable names was cherry-picked from that bigger PR, to separate trivial renames from code changes. - PR Review Comment: https://git.openjdk.org/jdk/pull/14653#discussion_r1242608041
Re: RFR: 8310890: Normalize identifier names
On Mon, 26 Jun 2023 14:07:03 GMT, Pavel Rappo wrote: > Please review this cleanup PR to normalize names of identifiers which are > Java variables/fields or tokens in text files. Those names either contain a > pronoun that is very rarely used in code, or seem like they contain such a > pronoun, which, in fact, they don't. Either way, the goal is to improve > readability and clarity. > > Also, this PR fixes a few related typos. make/data/charsetmapping/charsets line 149: > 147: package sun.nio.cs > 148: typesbcs > 149: histname ISO8859_2 Should this column be re-aligned with the longer name? - PR Review Comment: https://git.openjdk.org/jdk/pull/14653#discussion_r1242595069
Re: RFR: 8310890: Normalize identifier names
On Mon, 26 Jun 2023 14:07:03 GMT, Pavel Rappo wrote: > Please review this cleanup PR to normalize names of identifiers which are > Java variables/fields or tokens in text files. Those names either contain a > pronoun that is very rarely used in code, or seem like they contain such a > pronoun, which, in fact, they don't. Either way, the goal is to improve > readability and clarity. > > Also, this PR fixes a few related typos. Looks good, with or without the suggestion. src/java.base/share/classes/java/util/EnumMap.java line 690: > 688: Object otherValue = em.vals[i]; > 689: if (otherValue != ourValue && > 690: (otherValue == null || !otherValue.equals(ourValue))) Is this the same as java.util.Objects: `!Objects.equals(vals[i], em.vals[i]);` - Marked as reviewed by rriggs (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/14653#pullrequestreview-1499137712 PR Review Comment: https://git.openjdk.org/jdk/pull/14653#discussion_r1242585695
Re: RFR: 8310890: Normalize identifier names
On Mon, 26 Jun 2023 14:07:03 GMT, Pavel Rappo wrote: > Please review this cleanup PR to normalize names of identifiers which are > Java variables/fields or tokens in text files. Those names either contain a > pronoun that is very rarely used in code, or seem like they contain such a > pronoun, which, in fact, they don't. Either way, the goal is to improve > readability and clarity. > > Also, this PR fixes a few related typos. Looks fine to me - Marked as reviewed by naoto (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/14653#pullrequestreview-1499124651
RFR: 8310890: Normalize identifier names
Please review this cleanup PR to normalize names of identifiers which are Java variables/fields or tokens in text files. Those names either contain a pronoun that is very rarely used in code, or seem like they contain such a pronoun, which, in fact, they don't. Either way, the goal is to improve readability and clarity. Also, this PR fixes a few related typos. - Commit messages: - Improve further - Initial commit Changes: https://git.openjdk.org/jdk/pull/14653/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=14653&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8310890 Stats: 184 lines in 10 files changed: 0 ins; 0 del; 184 mod Patch: https://git.openjdk.org/jdk/pull/14653.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14653/head:pull/14653 PR: https://git.openjdk.org/jdk/pull/14653
