Re: RFR: 8276348: Use blessed modifier order in java.base
On 4/11/2021 12:14 am, Pavel Rappo wrote:
On Tue, 2 Nov 2021 16:30:56 GMT, Pavel Rappo wrote:
This PR follows up one of the recent PRs, where I used a non-canonical modifier
order. Since the problem was noticed [^1], why not to address it en masse?
As far as I remember, the first mass-canonicalization of modifiers took place
in JDK-8136583 in 2015 [^2]. That change affected 1780 lines spanning 453
files. Since then modifiers have become a bit loose, and it makes sense to
re-bless (using the JDK-8136583 terminology) them.
This change was produced by running the below command followed by updating the
copyright years on the affected files where necessary:
$ sh ./bin/blessed-modifier-order.sh src/java.base
The resulting change is much smaller than that of 2015: 39 lines spanning 21
files.
[^1]:
https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-November/082987.html
(or https://github.com/openjdk/jdk/pull/6191#pullrequestreview-79465)
[^2]:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-September/035217.html
_Mailing list message from [David Holmes](mailto:david.hol...@oracle.com) on
[core-libs-dev](mailto:core-libs-...@mail.openjdk.java.net):_
On 3/11/2021 9:26 pm, Pavel Rappo wrote:
On Tue, 2 Nov 2021 20:34:44 GMT, Martin Buchholz wrote:
Pragmatically, fix the script to ignore those keywords on comment lines. Learn
Perl, its just a regular expression pattern match and replace expression.
I understand in principle how to modify that script to ignore doc comments. The thing I
was referring to when said "btw, how would we do that?" was this: not all
comment lines are prose. Some of those lines belong to snippets of code, which I guess
you would also like to be properly formatted.
But having seen several reviewers be unmoved by the difference, the real
pragmatic view is to ignore the English.
I'm sorry you feel that way. Would it be okay if I made it clear that those two
words are not English adjectives but are special symbols that happen to use
Latin script and originate from the English words they resemble? If so, I could
enclose each of them in `{@code ... }`. If not, I could drop that particular
change from this PR.
The blessed-modifier-order.sh script intentionally modifies comments, with the
hope of finding code snippets (it did!)
Probably I manually deleted the change to Object.java back in 2015, to avoid
the sort of controversy we're seeing now.
I don't have a strong feeling either way on changing that file.
I agree with @pavelrappo that script-generated changes should not be mixed
with manual changes.
I would also not update copyright years for such changes.
It's a feature of blessed-modifier-order.sh that all existing formatting is
perfectly preserved.
One more thing. Please have a look at this other line in the same file; this
line was there before the change
https://github.com/openjdk/jdk/blob/465d350d0b3cac277a58b9f8ece196c1cde68e80/src/java.base/share/classes/java/lang/Object.java#L49
So before the change, the file was somewhat inconsistent. The change made it
consistent. **If one is going to ever revert that controversial part of the
change, please update both lines so that the file remains consistent.**
Line 281 is (was!) consistent with line 277 because it is distinguishing a synchronized "static method" from
a synchronized "instance method". There was no need to make any change to line 281 because of the
blessed-order of modifiers as defined for method declarations! This text is just prose. Now for consistency you should
change line 277 to refer to a "non-static synchronized method" (as "instance synchronized method"
would be truly awful).
Thanks, David. You've provided a clear and convincing argument, and I can see
the inconsistency I introduced. I can revert that particular piece back if you
think that it would be appropriate.
That said, this line will have to be filtered out every time the script is run.
I could probably provide a more involved script that harnesses the power of AST
(com.sun.source.doctree) to try to filter out prose, but it would be impossible
to beat the simplicity of the current script; and simplicity is also important.
Given this is prose, the adjectives should be separated by commas: "a
synchronized, static method", and "a synchronized, instance method".
Does that avoid the problem with the script?
Line 49 places "static synchronized" in code font, implying that it is referring to the actual method
modifiers and as such using the blessed order is appropriate. Line 49 does not need to be "consistent" with
line 281. If line 49 used a normal font so the words "static" and "synchronized" were just prose
then either order would be perfectly fine in terms of English language, but then you could make a case for having it be
consistent with line 281.
I've been always having hard time with modifiers being not enclosed in `@code`
in the first place; they are barely
Re: RFR: 8276348: Use blessed modifier order in java.base
On Tue, 2 Nov 2021 16:30:56 GMT, Pavel Rappo wrote:
> This PR follows up one of the recent PRs, where I used a non-canonical
> modifier order. Since the problem was noticed [^1], why not to address it en
> masse?
>
> As far as I remember, the first mass-canonicalization of modifiers took place
> in JDK-8136583 in 2015 [^2]. That change affected 1780 lines spanning 453
> files. Since then modifiers have become a bit loose, and it makes sense to
> re-bless (using the JDK-8136583 terminology) them.
>
> This change was produced by running the below command followed by updating
> the copyright years on the affected files where necessary:
>
> $ sh ./bin/blessed-modifier-order.sh src/java.base
>
> The resulting change is much smaller than that of 2015: 39 lines spanning 21
> files.
>
> [^1]:
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-November/082987.html
> (or https://github.com/openjdk/jdk/pull/6191#pullrequestreview-79465)
> [^2]:
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-September/035217.html
> _Mailing list message from [David Holmes](mailto:david.hol...@oracle.com) on
> [core-libs-dev](mailto:core-libs-...@mail.openjdk.java.net):_
>
> On 3/11/2021 9:26 pm, Pavel Rappo wrote:
>
> > On Tue, 2 Nov 2021 20:34:44 GMT, Martin Buchholz
> > wrote:
> > > > > Pragmatically, fix the script to ignore those keywords on comment
> > > > > lines. Learn Perl, its just a regular expression pattern match and
> > > > > replace expression.
> > > >
> > > >
> > > > I understand in principle how to modify that script to ignore doc
> > > > comments. The thing I was referring to when said "btw, how would we do
> > > > that?" was this: not all comment lines are prose. Some of those lines
> > > > belong to snippets of code, which I guess you would also like to be
> > > > properly formatted.
> > > > > But having seen several reviewers be unmoved by the difference, the
> > > > > real pragmatic view is to ignore the English.
> > > >
> > > >
> > > > I'm sorry you feel that way. Would it be okay if I made it clear that
> > > > those two words are not English adjectives but are special symbols that
> > > > happen to use Latin script and originate from the English words they
> > > > resemble? If so, I could enclose each of them in `{@code ... }`. If
> > > > not, I could drop that particular change from this PR.
> > >
> > >
> > > The blessed-modifier-order.sh script intentionally modifies comments,
> > > with the hope of finding code snippets (it did!)
> > > Probably I manually deleted the change to Object.java back in 2015, to
> > > avoid the sort of controversy we're seeing now.
> > > I don't have a strong feeling either way on changing that file.
> > > I agree with @pavelrappo that script-generated changes should not be
> > > mixed with manual changes.
> > > I would also not update copyright years for such changes.
> > > It's a feature of blessed-modifier-order.sh that all existing formatting
> > > is perfectly preserved.
> >
> >
> > One more thing. Please have a look at this other line in the same file;
> > this line was there before the change
> > https://github.com/openjdk/jdk/blob/465d350d0b3cac277a58b9f8ece196c1cde68e80/src/java.base/share/classes/java/lang/Object.java#L49
> > So before the change, the file was somewhat inconsistent. The change made
> > it consistent. **If one is going to ever revert that controversial part of
> > the change, please update both lines so that the file remains consistent.**
>
> Line 281 is (was!) consistent with line 277 because it is distinguishing a
> synchronized "static method" from a synchronized "instance method". There was
> no need to make any change to line 281 because of the blessed-order of
> modifiers as defined for method declarations! This text is just prose. Now
> for consistency you should change line 277 to refer to a "non-static
> synchronized method" (as "instance synchronized method" would be truly awful).
Thanks, David. You've provided a clear and convincing argument, and I can see
the inconsistency I introduced. I can revert that particular piece back if you
think that it would be appropriate.
That said, this line will have to be filtered out every time the script is run.
I could probably provide a more involved script that harnesses the power of AST
(com.sun.source.doctree) to try to filter out prose, but it would be impossible
to beat the simplicity of the current script; and simplicity is also important.
> Line 49 places "static synchronized" in code font, implying that it is
> referring to the actual method modifiers and as such using the blessed order
> is appropriate. Line 49 does not need to be "consistent" with line 281. If
> line 49 used a normal font so the words "static" and "synchronized" were just
> prose then either order would be perfectly fine in terms of English language,
> but then you could make a case for having it be consistent with line 281.
Re: RFR: 8276348: Use blessed modifier order in java.base
On 3/11/2021 9:26 pm, Pavel Rappo wrote:
On Tue, 2 Nov 2021 20:34:44 GMT, Martin Buchholz wrote:
Pragmatically, fix the script to ignore those keywords on comment lines. Learn
Perl, its just a regular expression pattern match and replace expression.
I understand in principle how to modify that script to ignore doc comments. The thing I
was referring to when said "btw, how would we do that?" was this: not all
comment lines are prose. Some of those lines belong to snippets of code, which I guess
you would also like to be properly formatted.
But having seen several reviewers be unmoved by the difference, the real
pragmatic view is to ignore the English.
I'm sorry you feel that way. Would it be okay if I made it clear that those two
words are not English adjectives but are special symbols that happen to use
Latin script and originate from the English words they resemble? If so, I could
enclose each of them in `{@code ... }`. If not, I could drop that particular
change from this PR.
The blessed-modifier-order.sh script intentionally modifies comments, with the
hope of finding code snippets (it did!)
Probably I manually deleted the change to Object.java back in 2015, to avoid
the sort of controversy we're seeing now.
I don't have a strong feeling either way on changing that file.
I agree with @pavelrappo that script-generated changes should not be mixed
with manual changes.
I would also not update copyright years for such changes.
It's a feature of blessed-modifier-order.sh that all existing formatting is
perfectly preserved.
One more thing. Please have a look at this other line in the same file; this
line was there before the change
https://github.com/openjdk/jdk/blob/465d350d0b3cac277a58b9f8ece196c1cde68e80/src/java.base/share/classes/java/lang/Object.java#L49
So before the change, the file was somewhat inconsistent. The change made it
consistent. **If one is going to ever revert that controversial part of the
change, please update both lines so that the file remains consistent.**
Line 281 is (was!) consistent with line 277 because it is distinguishing
a synchronized "static method" from a synchronized "instance method".
There was no need to make any change to line 281 because of the
blessed-order of modifiers as defined for method declarations! This text
is just prose. Now for consistency you should change line 277 to refer
to a "non-static synchronized method" (as "instance synchronized method"
would be truly awful).
Line 49 places "static synchronized" in code font, implying that it is
referring to the actual method modifiers and as such using the blessed
order is appropriate. Line 49 does not need to be "consistent" with line
281. If line 49 used a normal font so the words "static" and
"synchronized" were just prose then either order would be perfectly fine
in terms of English language, but then you could make a case for having
it be consistent with line 281.
Cheers,
David
-
-
PR: https://git.openjdk.java.net/jdk/pull/6213
Re: RFR: 8276348: Use blessed modifier order in java.base
On Tue, 2 Nov 2021 20:34:44 GMT, Martin Buchholz wrote:
>>> Pragmatically, fix the script to ignore those keywords on comment lines.
>>> Learn Perl, its just a regular expression pattern match and replace
>>> expression.
>>
>> I understand in principle how to modify that script to ignore doc comments.
>> The thing I was referring to when said "btw, how would we do that?" was
>> this: not all comment lines are prose. Some of those lines belong to
>> snippets of code, which I guess you would also like to be properly formatted.
>>
>>> But having seen several reviewers be unmoved by the difference, the real
>>> pragmatic view is to ignore the English.
>>
>> I'm sorry you feel that way. Would it be okay if I made it clear that those
>> two words are not English adjectives but are special symbols that happen to
>> use Latin script and originate from the English words they resemble? If so,
>> I could enclose each of them in `{@code ... }`. If not, I could drop that
>> particular change from this PR.
>
> The blessed-modifier-order.sh script intentionally modifies comments, with
> the hope of finding code snippets (it did!)
>
> Probably I manually deleted the change to Object.java back in 2015, to avoid
> the sort of controversy we're seeing now.
> I don't have a strong feeling either way on changing that file.
>
> I agree with @pavelrappo that script-generated changes should not be mixed
> with manual changes.
> I would also not update copyright years for such changes.
>
> It's a feature of blessed-modifier-order.sh that all existing formatting is
> perfectly preserved.
One more thing. Please have a look at this other line in the same file; this
line was there before the change
https://github.com/openjdk/jdk/blob/465d350d0b3cac277a58b9f8ece196c1cde68e80/src/java.base/share/classes/java/lang/Object.java#L49
So before the change, the file was somewhat inconsistent. The change made it
consistent. **If one is going to ever revert that controversial part of the
change, please update both lines so that the file remains consistent.**
-
PR: https://git.openjdk.java.net/jdk/pull/6213
Re: RFR: 8276348: Use blessed modifier order in java.base
On Tue, 2 Nov 2021 19:14:23 GMT, Pavel Rappo wrote:
>> Pragmatically, fix the script to ignore those keywords on comment lines.
>> Learn Perl, its just a regular expression pattern match and replace
>> expression.
>>
>> All of the changes have to be manually reviewed by the author and then the
>> reviewers.
>> Checking unneeded changes is part of every mechanical change.
>>
>> The text being changed in the javadoc is the *spec*; that deserves special
>> attention in review.
>>
>> But having seen several reviewers be unmoved by the difference, the real
>> pragmatic view
>> is to ignore the English.
>
>> Pragmatically, fix the script to ignore those keywords on comment lines.
>> Learn Perl, its just a regular expression pattern match and replace
>> expression.
>
> I understand in principle how to modify that script to ignore doc comments.
> The thing I was referring to when said "btw, how would we do that?" was this:
> not all comment lines are prose. Some of those lines belong to snippets of
> code, which I guess you would also like to be properly formatted.
>
>> But having seen several reviewers be unmoved by the difference, the real
>> pragmatic view is to ignore the English.
>
> I'm sorry you feel that way. Would it be okay if I made it clear that those
> two words are not English adjectives but are special symbols that happen to
> use Latin script and originate from the English words they resemble? If so, I
> could enclose each of them in `{@code ... }`. If not, I could drop that
> particular change from this PR.
The blessed-modifier-order.sh script intentionally modifies comments, with the
hope of finding code snippets (it did!)
Probably I manually deleted the change to Object.java back in 2015, to avoid
the sort of controversy we're seeing now.
I don't have a strong feeling either way on changing that file.
I agree with @pavelrappo that script-generated changes should not be mixed
with manual changes.
I would also not update copyright years for such changes.
It's a feature of blessed-modifier-order.sh that all existing formatting is
perfectly preserved.
-
PR: https://git.openjdk.java.net/jdk/pull/6213
Re: RFR: 8276348: Use blessed modifier order in java.base
On Tue, 2 Nov 2021 16:30:56 GMT, Pavel Rappo wrote: > This PR follows up one of the recent PRs, where I used a non-canonical > modifier order. Since the problem was noticed [^1], why not to address it at > mass? > > As far as I remember, the first mass-canonicalization of modifiers took place > in JDK-8136583 in 2015 [^2]. That change affected 1780 lines spanning 453 > files. Since then modifiers have become a bit loose, and it makes sense to > re-bless (using the JDK-8136583 terminology) them. > > This change was produced by running the below command followed by updating > the copyright years on the affected files where necessary: > > $ sh ./bin/blessed-modifier-order.sh src/java.base > > The resulting change is much smaller than that of 2015: 39 lines spanning 21 > files. > > [^1]: > https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-November/082987.html > (or https://github.com/openjdk/jdk/pull/6191#pullrequestreview-79465) > [^2]: > http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-September/035217.html Marked as reviewed by martin (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6213
Re: RFR: 8276348: Use blessed modifier order in java.base
On Tue, 2 Nov 2021 19:22:15 GMT, Pavel Rappo wrote:
>> src/java.base/share/classes/java/lang/invoke/CallSite.java line 88:
>>
>>> 86: */
>>> 87: public
>>> 88: abstract class CallSite {
>>
>> I think it's better to move all modifiers to the single line.
>
> This is a survivorship bias. This example jumps out at you, because it
> happens to use missorted modifiers. I'm pretty sure this is not an
> aberration, but a style. If you want to change that style, you should create
> a respective JBS issue.
I've grepped the code and can now see that a list of modifiers broken by
newlines which cannot be explained by line-width concerns is indeed an
irregularity. Not only in java.base but also in other modules. Although there
aren't many of such cases, I would prefer them to be addressed in a separate
cleanup, which you are welcome to pursue.
-
PR: https://git.openjdk.java.net/jdk/pull/6213
Re: RFR: 8276348: Use blessed modifier order in java.base
On Tue, 2 Nov 2021 16:30:56 GMT, Pavel Rappo wrote: > This PR follows up one of the recent PRs, where I used a non-canonical > modifier order. Since the problem was noticed [^1], why not to address it at > mass? > > As far as I remember, the first mass-canonicalization of modifiers took place > in JDK-8136583 in 2015 [^2]. That change affected 1780 lines spanning 453 > files. Since then modifiers have become a bit loose, and it makes sense to > re-bless (using the JDK-8136583 terminology) them. > > This change was produced by running the below command followed by updating > the copyright years on the affected files where necessary: > > $ sh ./bin/blessed-modifier-order.sh src/java.base > > The resulting change is much smaller than that of 2015: 39 lines spanning 21 > files. > > [^1]: > https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-November/082987.html > (or https://github.com/openjdk/jdk/pull/6191#pullrequestreview-79465) > [^2]: > http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-September/035217.html Keep it as is with the modifiers in the recommended order. I don't think adding extra typography is warranted. - Marked as reviewed by rriggs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6213
Re: RFR: 8276348: Use blessed modifier order in java.base
On Tue, 2 Nov 2021 19:15:26 GMT, Andrey Turbanov wrote:
>> This PR follows up one of the recent PRs, where I used a non-canonical
>> modifier order. Since the problem was noticed [^1], why not to address it at
>> mass?
>>
>> As far as I remember, the first mass-canonicalization of modifiers took
>> place in JDK-8136583 in 2015 [^2]. That change affected 1780 lines spanning
>> 453 files. Since then modifiers have become a bit loose, and it makes sense
>> to re-bless (using the JDK-8136583 terminology) them.
>>
>> This change was produced by running the below command followed by updating
>> the copyright years on the affected files where necessary:
>>
>> $ sh ./bin/blessed-modifier-order.sh src/java.base
>>
>> The resulting change is much smaller than that of 2015: 39 lines spanning 21
>> files.
>>
>> [^1]:
>> https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-November/082987.html
>> (or https://github.com/openjdk/jdk/pull/6191#pullrequestreview-79465)
>> [^2]:
>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-September/035217.html
>
> src/java.base/share/classes/java/lang/invoke/CallSite.java line 88:
>
>> 86: */
>> 87: public
>> 88: abstract class CallSite {
>
> I think it's better to move all modifiers to the single line.
This is a survivorship bias. This example jumps out at you, because it happens
to use missorted modifiers. I'm pretty sure this is not an aberration, but a
style. If you want to change that style, you should create a respective JBS
issue.
-
PR: https://git.openjdk.java.net/jdk/pull/6213
Re: RFR: 8276348: Use blessed modifier order in java.base
On Tue, 2 Nov 2021 16:30:56 GMT, Pavel Rappo wrote:
> This PR follows up one of the recent PRs, where I used a non-canonical
> modifier order. Since the problem was noticed [^1], why not to address it at
> mass?
>
> As far as I remember, the first mass-canonicalization of modifiers took place
> in JDK-8136583 in 2015 [^2]. That change affected 1780 lines spanning 453
> files. Since then modifiers have become a bit loose, and it makes sense to
> re-bless (using the JDK-8136583 terminology) them.
>
> This change was produced by running the below command followed by updating
> the copyright years on the affected files where necessary:
>
> $ sh ./bin/blessed-modifier-order.sh src/java.base
>
> The resulting change is much smaller than that of 2015: 39 lines spanning 21
> files.
>
> [^1]:
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-November/082987.html
> (or https://github.com/openjdk/jdk/pull/6191#pullrequestreview-79465)
> [^2]:
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-September/035217.html
src/java.base/share/classes/java/lang/invoke/CallSite.java line 88:
> 86: */
> 87: public
> 88: abstract class CallSite {
I think it's better to move all modifiers to the single line.
-
PR: https://git.openjdk.java.net/jdk/pull/6213
Re: RFR: 8276348: Use blessed modifier order in java.base
On Tue, 2 Nov 2021 18:48:20 GMT, Roger Riggs wrote:
> Pragmatically, fix the script to ignore those keywords on comment lines.
> Learn Perl, its just a regular expression pattern match and replace
> expression.
I understand in principle how to modify that script to ignore doc comments. The
thing I was referring to when said "btw, how would we do that?" was this: not
all comment lines are prose. Some of those lines belong to snippets of code,
which I guess you would also like to be properly formatted.
> But having seen several reviewers be unmoved by the difference, the real
> pragmatic view is to ignore the English.
I'm sorry you feel that way. Would it be okay if I made it clear that those two
words are not English adjectives but are special symbols that happen to use
Latin script and originate from the English words they resemble? If so, I could
enclose each of them in `{@code ... }`. If not, I could drop that particular
change from this PR.
-
PR: https://git.openjdk.java.net/jdk/pull/6213
Re: RFR: 8276348: Use blessed modifier order in java.base
On Tue, 2 Nov 2021 18:17:36 GMT, Pavel Rappo wrote: >> It's tough when a natural language clashes with a programming language. I >> appreciate that this particular clash might cause discomfort to native >> English speakers. (This reminds me of that _DOSASCOMP_ mnemonic for >> adjective order.) That said, consider the following pragmatic aspect. Unless >> we change the script not to process prose in comments (btw, how would we do >> that?), every single time we run that script, that particular line in >> Object.java will need to be tracked and excluded. > > Here's a bit of archaeology. I found the original JDK-8136583 patch, which > has moved from where it was in the RFR to > https://cr.openjdk.java.net/~martin/webrevs/jdk9/blessed-modifier-order/. > That patch doesn't change Object.java. The RFR thread mentions neither that > javadoc change nor any javadoc change for that matter. So either the script > was different, or Martin filtered that line (from Object.java) out before > creating the webrev. > > Now, in his followup thread on core-libs-dev, > http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-September/035273.html, > Martin specifically pointed out javadoc changes and said that they seem fine > to him. "to each his own". I think static synchronized reads best and more natural than synchronzied static. Also from a semantic point of view as it conveys class level characteristic thus lends static to having a prominence in specified order. - PR: https://git.openjdk.java.net/jdk/pull/6213
Re: RFR: 8276348: Use blessed modifier order in java.base
On Tue, 2 Nov 2021 18:48:06 GMT, Mark Sheppard wrote: >> Here's a bit of archaeology. I found the original JDK-8136583 patch, which >> has moved from where it was in the RFR to >> https://cr.openjdk.java.net/~martin/webrevs/jdk9/blessed-modifier-order/. >> That patch doesn't change Object.java. The RFR thread mentions neither that >> javadoc change nor any javadoc change for that matter. So either the script >> was different, or Martin filtered that line (from Object.java) out before >> creating the webrev. >> >> Now, in his followup thread on core-libs-dev, >> http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-September/035273.html, >> Martin specifically pointed out javadoc changes and said that they seem >> fine to him. > > "to each his own". I think static synchronized reads best and more natural > than synchronzied static. Also from a semantic point of view as it conveys > class level characteristic thus lends static to having a prominence in > specified order. Pragmatically, fix the script to ignore those keywords on comment lines. Learn Perl, its just a regular expression pattern match and replace expression. All of the changes have to be manually reviewed by the author and then the reviewers. Checking unneeded changes is part of every mechanical change. The text being changed in the javadoc is the *spec*; that deserves special attention in review. But having seen several reviewers be unmoved by the difference, the real pragmatic view is to ignore the English. - PR: https://git.openjdk.java.net/jdk/pull/6213
Re: RFR: 8276348: Use blessed modifier order in java.base
On Tue, 2 Nov 2021 16:30:56 GMT, Pavel Rappo wrote: > This PR follows up one of the recent PRs, where I used a non-canonical > modifier order. Since the problem was noticed [^1], why not to address it at > mass? > > As far as I remember, the first mass-canonicalization of modifiers took place > in JDK-8136583 in 2015 [^2]. That change affected 1780 lines spanning 453 > files. Since then modifiers have become a bit lose, and it makes sense to > re-bless (using the JDK-8136583 terminology) them. > > This change was produced by running the below command followed by updating > the copyright years on the affected files where necessary: > > $ sh ./bin/blessed-modifier-order.sh src/java.base > > The resulting change is much smaller than that of 2015: 39 lines spanning 21 > files. > > [^1]: > https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-November/082987.html > (or https://github.com/openjdk/jdk/pull/6191#pullrequestreview-79465) > [^2]: > http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-September/035217.html JFYI a couple of times I've wondered if we regressed on this. I just ran the script on the desktop module and we havea few instances there too, so I've filed a clean up bug on it. - PR: https://git.openjdk.java.net/jdk/pull/6213
Re: RFR: 8276348: Use blessed modifier order in java.base
On Tue, 2 Nov 2021 17:45:07 GMT, Pavel Rappo wrote: >>> In comments, I think the 'synchronized static 'reads better, the >>> conventional order is for the code. >> >> I think Roger is right and maybe the change to the javadoc could be dropped >> from this patch. > > It's tough when a natural language clashes with a programming language. I > appreciate that this particular clash might cause discomfort to native > English speakers. (This reminds me of that _DOSASCOMP_ mnemonic for adjective > order.) That said, consider the following pragmatic aspect. Unless we change > the script not to process prose in comments (btw, how would we do that?), > every single time we run that script, that particular line in Object.java > will need to be tracked and excluded. Here's a bit of archaeology. I found the original JDK-8136583 patch, which has moved from where it was in the RFR to https://cr.openjdk.java.net/~martin/webrevs/jdk9/blessed-modifier-order/. That patch doesn't change Object.java. The RFR thread mentions neither that javadoc change nor any javadoc change for that matter. So either the script was different, or Martin filtered that line (from Object.java) out before creating the webrev. Now, in his followup thread on core-libs-dev, http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-September/035273.html, Martin specifically pointed out javadoc changes and said that they seem fine to him. - PR: https://git.openjdk.java.net/jdk/pull/6213
Re: RFR: 8276348: Use blessed modifier order in java.base
On Tue, 2 Nov 2021 17:39:17 GMT, Alan Bateman wrote:
>> src/java.base/share/classes/java/lang/Object.java line 282:
>>
>>> 280: * For objects of type {@code Class,} by executing a
>>> 281: * static synchronized method of that class.
>>> 282: *
>>
>> In comments, I think the 'synchronized static 'reads better, the
>> conventional order is for the code.
>
>> In comments, I think the 'synchronized static 'reads better, the
>> conventional order is for the code.
>
> I think Roger is right and maybe the change to the javadoc could be dropped
> from this patch.
It's tough when a natural language clashes with a programming language. I
appreciate that this particular clash might cause discomfort to native English
speakers. (This reminds me of that _DOSASCOMP_ mnemonic for adjective order.)
That said, consider the following pragmatic aspect. Unless we change the script
not to process prose in comments (btw, how would we do that?), every single
time we run that script, that particular line in Object.java will need to be
tracked and excluded.
-
PR: https://git.openjdk.java.net/jdk/pull/6213
Re: RFR: 8276348: Use blessed modifier order in java.base
On Tue, 2 Nov 2021 17:13:47 GMT, Roger Riggs wrote: > In comments, I think the 'synchronized static 'reads better, the conventional > order is for the code. I think Roger is right and maybe the change to the javadoc could be dropped from this patch. - PR: https://git.openjdk.java.net/jdk/pull/6213
Re: RFR: 8276348: Use blessed modifier order in java.base
On Tue, 2 Nov 2021 16:30:56 GMT, Pavel Rappo wrote: > This PR follows up one of the recent PRs, where I used a non-canonical > modifier order. Since the problem was noticed [^1], why not to address it at > mass? > > As far as I remember, the first mass-canonicalization of modifiers took place > in JDK-8136583 in 2015 [^2]. That change affected 1780 lines spanning 453 > files. Since then modifiers have become a bit lose, and it makes sense to > re-bless (using the JDK-8136583 terminology) them. > > This change was produced by running the below command followed by updating > the copyright years on the affected files where necessary: > > $ sh ./bin/blessed-modifier-order.sh src/java.base > > The resulting change is much smaller than that of 2015: 39 lines spanning 21 > files. > > [^1]: > https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-November/082987.html > (or https://github.com/openjdk/jdk/pull/6191#pullrequestreview-79465) > [^2]: > http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-September/035217.html Marked as reviewed by darcy (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6213
Re: RFR: 8276348: Use blessed modifier order in java.base
On Tue, 2 Nov 2021 16:30:56 GMT, Pavel Rappo wrote: > This PR follows up one of the recent PRs, where I used a non-canonical > modifier order. Since the problem was noticed [^1], why not to address it at > mass? > > As far as I remember, the first mass-canonicalization of modifiers took place > in JDK-8136583 in 2015 [^2]. That change affected 1780 lines spanning 453 > files. Since then modifiers have become a bit lose, and it makes sense to > re-bless (using the JDK-8136583 terminology) them. > > This change was produced by running the below command followed by updating > the copyright years on the affected files where necessary: > > $ sh ./bin/blessed-modifier-order.sh src/java.base > > The resulting change is much smaller than that of 2015: 39 lines spanning 21 > files. > > [^1]: > https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-November/082987.html > (or https://github.com/openjdk/jdk/pull/6191#pullrequestreview-79465) > [^2]: > http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-September/035217.html Marked as reviewed by iris (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6213
Re: RFR: 8276348: Use blessed modifier order in java.base
On Tue, 2 Nov 2021 16:30:56 GMT, Pavel Rappo wrote:
> This PR follows up one of the recent PRs, where I used a non-canonical
> modifier order. Since the problem was noticed [^1], why not to address it at
> mass?
>
> As far as I remember, the first mass-canonicalization of modifiers took place
> in JDK-8136583 in 2015 [^2]. That change affected 1780 lines spanning 453
> files. Since then modifiers have become a bit lose, and it makes sense to
> re-bless (using the JDK-8136583 terminology) them.
>
> This change was produced by running the below command followed by updating
> the copyright years on the affected files where necessary:
>
> $ sh ./bin/blessed-modifier-order.sh src/java.base
>
> The resulting change is much smaller than that of 2015: 39 lines spanning 21
> files.
>
> [^1]:
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-November/082987.html
> (or https://github.com/openjdk/jdk/pull/6191#pullrequestreview-79465)
> [^2]:
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-September/035217.html
src/java.base/share/classes/java/lang/Object.java line 282:
> 280: * For objects of type {@code Class,} by executing a
> 281: * static synchronized method of that class.
> 282: *
In comments, I think the 'synchronized static 'reads better, the conventional
order is for the code.
-
PR: https://git.openjdk.java.net/jdk/pull/6213
Re: RFR: 8276348: Use blessed modifier order in java.base
On Tue, 2 Nov 2021 16:30:56 GMT, Pavel Rappo wrote: > This PR follows up one of the recent PRs, where I used a non-canonical > modifier order. Since the problem was noticed [^1], why not to address it at > mass? > > As far as I remember, the first mass-canonicalization of modifiers took place > in JDK-8136583 in 2015 [^2]. That change affected 1780 lines spanning 453 > files. Since then modifiers have become a bit lose, and it makes sense to > re-bless (using the JDK-8136583 terminology) them. > > This change was produced by running the below command followed by updating > the copyright years on the affected files where necessary: > > $ sh ./bin/blessed-modifier-order.sh src/java.base > > The resulting change is much smaller than that of 2015: 39 lines spanning 21 > files. > > [^1]: > https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-November/082987.html > (or https://github.com/openjdk/jdk/pull/6191#pullrequestreview-79465) > [^2]: > http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-September/035217.html Marked as reviewed by dfuchs (Reviewer). LGTM - PR: https://git.openjdk.java.net/jdk/pull/6213
Re: RFR: 8276348: Use blessed modifier order in java.base
On Tue, 2 Nov 2021 16:30:56 GMT, Pavel Rappo wrote: > This PR follows up one of the recent PRs, where I used a non-canonical > modifier order. Since the problem was noticed [^1], why not to address it at > mass? > > As far as I remember, the first mass-canonicalization of modifiers took place > in JDK-8136583 in 2015 [^2]. That change affected 1780 lines spanning 453 > files. Since then modifiers have become a bit lose, and it makes sense to > re-bless (using the JDK-8136583 terminology) them. > > This change was produced by running the below command followed by updating > the copyright years on the affected files where necessary: > > $ sh ./bin/blessed-modifier-order.sh src/java.base > > The resulting change is much smaller than that of 2015: 39 lines spanning 21 > files. > > [^1]: > https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-November/082987.html > (or https://github.com/openjdk/jdk/pull/6191#pullrequestreview-79465) > [^2]: > http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-September/035217.html A colleague suggested that I should clarify that the `blessed-modifier-order.sh` script that I used is available in the JDK repo at https://github.com/openjdk/jdk/blob/01105d6985b39d4064b9066eab3612da5a401685/bin/blessed-modifier-order.sh. That script was contributed by Martin Buchholz in JDK-8136656 in 2015. - PR: https://git.openjdk.java.net/jdk/pull/6213
RFR: 8276348: Use blessed modifier order in java.base
This PR follows up one of the recent PRs, where I used a non-canonical modifier order. Since the problem was noticed [^1], why not to address it at mass? As far as I remember, the first mass-canonicalization of modifiers took place in JDK-8136583 in 2015 [^2]. That change affected 1780 lines spanning 453 files. Since then modifiers have become a bit lose, and it makes sense to re-bless (using the JDK-8136583 terminology) them. This change was produced by running the below command followed by updating the copyright years on the affected files where necessary: $ sh ./bin/blessed-modifier-order.sh src/java.base The resulting change is much smaller than that of 2015: 39 lines spanning 21 files. [^1]: https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-November/082987.html (or https://github.com/openjdk/jdk/pull/6191#pullrequestreview-79465) [^2]: http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-September/035217.html - Commit messages: - Initial commit Changes: https://git.openjdk.java.net/jdk/pull/6213/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6213=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8276348 Stats: 39 lines in 21 files changed: 0 ins; 0 del; 39 mod Patch: https://git.openjdk.java.net/jdk/pull/6213.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6213/head:pull/6213 PR: https://git.openjdk.java.net/jdk/pull/6213
