Re: RFR: 8263658: Use the blessed modifier order in java.base
On Fri, 19 Mar 2021 18:31:02 GMT, Naoto Sato wrote: >> Additional changes found in `java.base` of `final private` -> `private >> final`. Filed with existing bug because it's the same module; can change to >> a different bug number if required. > > Marked as reviewed by naoto (Reviewer). Can't reuse the bug IDs, sadly. Filed https://bugs.openjdk.java.net/browse/JDK-8263892 (note the new summary name to disambiguate) - PR: https://git.openjdk.java.net/jdk/pull/3094
Re: RFR: 8263658: Use the blessed modifier order in java.base
On Fri, 19 Mar 2021 18:23:00 GMT, Alex Blewitt wrote: > Additional changes found in `java.base` of `final private` -> `private > final`. Filed with existing bug because it's the same module; can change to a > different bug number if required. Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3094
RFR: 8263658: Use the blessed modifier order in java.base
Additional changes found in `java.base` of `final private` -> `private final`. Filed with existing bug because it's the same module; can change to a different bug number if required. - Commit messages: - 8263658: Use the blessed modifier order in java.base Changes: https://git.openjdk.java.net/jdk/pull/3094/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3094=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8263658 Stats: 19 lines in 5 files changed: 0 ins; 0 del; 19 mod Patch: https://git.openjdk.java.net/jdk/pull/3094.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3094/head:pull/3094 PR: https://git.openjdk.java.net/jdk/pull/3094
Re: RFR: 8263658: Use the blessed modifier order in java.base [v2]
On Thu, 18 Mar 2021 17:02:57 GMT, Alex Blewitt wrote: >> Sonar displays a warning message that modifiers should be declared in the >> order listed in the JLS; specifically, that isntead of using `final static` >> the `static final` should be preferred. >> >> This fixes the issues in the `java.base` package for ease of reviewing. >> >> https://sonarcloud.io/project/issues?id=shipilev_jdk=java=false=java%3AS1124 > > Alex Blewitt has refreshed the contents of this pull request, and previous > commits have been removed. The incremental views will show differences > compared to the previous content of the PR. Marked as reviewed by redestad (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2993
Re: RFR: 8263658: Use the blessed modifier order in java.base [v2]
On Thu, 18 Mar 2021 17:06:04 GMT, Alex Blewitt wrote: >>> If I have other fixes for different modules, should I file PRs with the >>> same bug number e.g. "8263658: Use the blessed modifier order in >>> java.logging/java.desktop" or should we have separate bug numbers for them? >> >> Separate bug numbers is necessary. Unless you repurpose / widen this PR to >> include more modules. >> >> A word of advice: Avoid git rebase + force push after opening a PR for >> review, since it might mess up the review context. Preferably either merge >> in changes from main, or just keep adding commits without syncing up - the >> system will ensure your patch can be merged in without conflicts. > > I'm happy to either widen the scope of this PR or to submit multiple but I > have to rely on the kindness of strangers to create appropriate bugs in the > system. On the one hand I don't want to cause a lot of noise but on the > other having smaller independent commits (particularly if they hit a lot of > files/modules) seems like a more sensible plan to me. There's some extra churn in splitting it up, sure, but different modules are often maintained by different people so getting changes that span the entire JDK reviewed might actually take you longer. YMMV. I can assist creating RFEs. - PR: https://git.openjdk.java.net/jdk/pull/2993
Re: RFR: 8263658: Use the blessed modifier order in java.base [v2]
On Thu, 18 Mar 2021 16:51:35 GMT, Alex Blewitt wrote: > If I have other fixes for different modules, should I file PRs with the same > bug number e.g. "8263658: Use the blessed modifier order in > java.logging/java.desktop" or should we have separate bug numbers for them? Separate bug numbers is necessary. Unless you repurpose / widen this PR to include more modules. A word of advice: Avoid git rebase + force push after opening a PR for review, since it might mess up the review context. Preferably either merge in changes from main, or just keep adding commits without syncing up - the system will ensure your patch can be merged in without conflicts. - PR: https://git.openjdk.java.net/jdk/pull/2993
Re: RFR: 8263658: Use the blessed modifier order in java.base [v2]
On Thu, 18 Mar 2021 17:03:28 GMT, Claes Redestad wrote: >> If I have other fixes for different modules, should I file PRs with the same >> bug number e.g. "8263658: Use the blessed modifier order in >> java.logging/java.desktop" or should we have separate bug numbers for them? > >> If I have other fixes for different modules, should I file PRs with the same >> bug number e.g. "8263658: Use the blessed modifier order in >> java.logging/java.desktop" or should we have separate bug numbers for them? > > Separate bug numbers is necessary. Unless you repurpose / widen this PR to > include more modules. > > A word of advice: Avoid git rebase + force push after opening a PR for > review, since it might mess up the review context. Preferably either merge in > changes from main, or just keep adding commits without syncing up - the > system will ensure your patch can be merged in without conflicts. I'm happy to either widen the scope of this PR or to submit multiple but I have to rely on the kindness of strangers to create appropriate bugs in the system. On the one hand I don't want to cause a lot of noise but on the other having smaller independent commits (particularly if they hit a lot of files/modules) seems like a more sensible plan to me. - PR: https://git.openjdk.java.net/jdk/pull/2993
Re: RFR: 8263658: Use the blessed modifier order in java.base [v2]
On Thu, 18 Mar 2021 16:50:39 GMT, Claes Redestad wrote: >> Is that there to indicate a placeholder value that was once used and is kept >> for documentation purposes? Should the corresponding JavaDoc be removed as >> well? Should I do this in the same commit/PR as this one, or submit a new >> PR? Would prefer to avoid conflating fixes if possible so that if one needs >> to be reverted we don't revert the related changes. > > There's another constant with value 3 defined, so I think this is just some > left-over. > > If you prefer separating out the removal to another RFE I'd remove this > particular change from this PR. Filed #3076 for the removal and updated this PR without it - PR: https://git.openjdk.java.net/jdk/pull/2993
Re: RFR: 8263658: Use the blessed modifier order in java.base [v2]
> Sonar displays a warning message that modifiers should be declared in the > order listed in the JLS; specifically, that isntead of using `final static` > the `static final` should be preferred. > > This fixes the issues in the `java.base` package for ease of reviewing. > > https://sonarcloud.io/project/issues?id=shipilev_jdk=java=false=java%3AS1124 Alex Blewitt has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: 8263658: Use the blessed modifier order in java.base - Changes: - all: https://git.openjdk.java.net/jdk/pull/2993/files - new: https://git.openjdk.java.net/jdk/pull/2993/files/470f7066..86aa9a34 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2993=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2993=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2993.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2993/head:pull/2993 PR: https://git.openjdk.java.net/jdk/pull/2993
Re: RFR: 8263658: Use the blessed modifier order in java.base
On 3/18/2021 9:53 AM, Alex Blewitt wrote: On Thu, 18 Mar 2021 14:50:43 GMT, Claes Redestad wrote: Sonar displays a warning message that modifiers should be declared in the order listed in the JLS; specifically, that isntead of using `final static` the `static final` should be preferred. This fixes the issues in the `java.base` package for ease of reviewing. https://sonarcloud.io/project/issues?id=shipilev_jdk=java=false=java%3AS1124 Marked as reviewed by redestad (Reviewer). If I have other fixes for different modules, should I file PRs with the same bug number e.g. "8263658: Use the blessed modifier order in java.logging/java.desktop" or should we have separate bug numbers for them? Separate bug numbers please. It is also possible to use an umbrella issue and have issues for, say each module, be subtasks. -Joe
Re: RFR: 8263658: Use the blessed modifier order in java.base
On Thu, 18 Mar 2021 16:42:39 GMT, Alex Blewitt wrote: >> Yeah, I agree. > > Is that there to indicate a placeholder value that was once used and is kept > for documentation purposes? Should the corresponding JavaDoc be removed as > well? Should I do this in the same commit/PR as this one, or submit a new PR? > Would prefer to avoid conflating fixes if possible so that if one needs to be > reverted we don't revert the related changes. There's another constant with value 3 defined, so I think this is just some left-over. If you prefer separating out the removal to another RFE I'd remove this particular change from this PR. - PR: https://git.openjdk.java.net/jdk/pull/2993
Re: RFR: 8263658: Use the blessed modifier order in java.base
On Thu, 18 Mar 2021 14:50:43 GMT, Claes Redestad wrote: >> Sonar displays a warning message that modifiers should be declared in the >> order listed in the JLS; specifically, that isntead of using `final static` >> the `static final` should be preferred. >> >> This fixes the issues in the `java.base` package for ease of reviewing. >> >> https://sonarcloud.io/project/issues?id=shipilev_jdk=java=false=java%3AS1124 > > Marked as reviewed by redestad (Reviewer). If I have other fixes for different modules, should I file PRs with the same bug number e.g. "8263658: Use the blessed modifier order in java.logging/java.desktop" or should we have separate bug numbers for them? - PR: https://git.openjdk.java.net/jdk/pull/2993
Re: RFR: 8263658: Use the blessed modifier order in java.base
On Thu, 18 Mar 2021 15:08:09 GMT, Aleksey Shipilev wrote: >> src/java.base/share/classes/com/sun/security/ntlm/NTLMException.java line 52: >> >>> 50: * from server. >>> 51: */ >>> 52: //public static final int DOMAIN_UNMATCH = 3; >> >> Maybe this one ought to be removed instead? > > Yeah, I agree. Is that there to indicate a placeholder value that was once used and is kept for documentation purposes? Should the corresponding JavaDoc be removed as well? Should I do this in the same commit/PR as this one, or submit a new PR? Would prefer to avoid conflating fixes if possible so that if one needs to be reverted we don't revert the related changes. - PR: https://git.openjdk.java.net/jdk/pull/2993
Re: RFR: 8263658: Use the blessed modifier order in java.base
On Wed, 17 Mar 2021 12:31:22 GMT, Claes Redestad wrote: >> Sonar displays a warning message that modifiers should be declared in the >> order listed in the JLS; specifically, that isntead of using `final static` >> the `static final` should be preferred. >> >> This fixes the issues in the `java.base` package for ease of reviewing. >> >> https://sonarcloud.io/project/issues?id=shipilev_jdk=java=false=java%3AS1124 > > src/java.base/share/classes/com/sun/security/ntlm/NTLMException.java line 52: > >> 50: * from server. >> 51: */ >> 52: //public static final int DOMAIN_UNMATCH = 3; > > Maybe this one ought to be removed instead? Yeah, I agree. - PR: https://git.openjdk.java.net/jdk/pull/2993
Re: RFR: 8263658: Use the blessed modifier order in java.base
On Sat, 13 Mar 2021 22:45:30 GMT, Alex Blewitt wrote: > Sonar displays a warning message that modifiers should be declared in the > order listed in the JLS; specifically, that isntead of using `final static` > the `static final` should be preferred. > > This fixes the issues in the `java.base` package for ease of reviewing. > > https://sonarcloud.io/project/issues?id=shipilev_jdk=java=false=java%3AS1124 Marked as reviewed by redestad (Reviewer). src/java.base/share/classes/com/sun/security/ntlm/NTLMException.java line 52: > 50: * from server. > 51: */ > 52: //public static final int DOMAIN_UNMATCH = 3; Maybe this one ought to be removed instead? - PR: https://git.openjdk.java.net/jdk/pull/2993
Re: RFR: 8263658: Use the blessed modifier order in java.base
On Sat, 13 Mar 2021 22:45:30 GMT, Alex Blewitt wrote: > Sonar displays a warning message that modifiers should be declared in the > order listed in the JLS; specifically, that isntead of using `final static` > the `static final` should be preferred. > > This fixes the issues in the `java.base` package for ease of reviewing. > > https://sonarcloud.io/project/issues?id=shipilev_jdk=java=false=java%3AS1124 Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2993
RFR: 8263658: Use the blessed modifier order in java.base
Sonar displays a warning message that modifiers should be declared in the order listed in the JLS; specifically, that isntead of using `final static` the `static final` should be preferred. This fixes the issues in the `java.base` package for ease of reviewing. https://sonarcloud.io/project/issues?id=shipilev_jdk=java=false=java%3AS1124 - Commit messages: - 8263658: Use the blessed modifier order in java.base Changes: https://git.openjdk.java.net/jdk/pull/2993/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2993=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8263658 Stats: 140 lines in 29 files changed: 0 ins; 0 del; 140 mod Patch: https://git.openjdk.java.net/jdk/pull/2993.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2993/head:pull/2993 PR: https://git.openjdk.java.net/jdk/pull/2993
Re: RFR: 8263658: Use the blessed modifier order in java.base
On Sat, 13 Mar 2021 22:45:30 GMT, Alex Blewitt wrote: > Sonar displays a warning message that modifiers should be declared in the > order listed in the JLS; specifically, that isntead of using `final static` > the `static final` should be preferred. > > This fixes the issues in the `java.base` package for ease of reviewing. > > https://sonarcloud.io/project/issues?id=shipilev_jdk=java=false=java%3AS1124 Please change the synopsis to "8263658: Use the blessed modifier order in java.base" to get this PR hooked properly to the new bug. Also configure the run the testing, see "Pre-submit test status" in "Checks". - PR: https://git.openjdk.java.net/jdk/pull/2993
