Re: RFR: 8332071: Convert package.html files in `java.management.rmi` to package-info.java [v5]
On Mon, 20 May 2024 20:55:16 GMT, Nizar Benalla wrote: >> Please review this change. I converted the `package.html` file to >> `package-info.java`, because `javac` cannot recognize `package.html`. >> I already brought this up [in the mailing >> list](https://mail.openjdk.org/pipermail/serviceability-dev/2024-May/055650.html). >> The conversion was done in-place, only renaming it in git. >> >> I also added a couple of `@since` tags, with only 2 changes I don't want to >> split these two fixes into separate PRs. >> `CREDENTIALS_FILTER_PATTERN` and `SERIAL_FILTER_PATTERN` were first added in >> https://bugs.openjdk.org/browse/JDK-8187556 > > Nizar Benalla has updated the pull request incrementally with one additional > commit since the last revision: > > fix indentation LGTM - Marked as reviewed by rriggs (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19263#pullrequestreview-2071636919
Re: RFR: 8332071: Convert package.html files in `java.management.rmi` to package-info.java [v5]
> Please review this change. I converted the `package.html` file to > `package-info.java`, because `javac` cannot recognize `package.html`. > I already brought this up [in the mailing > list](https://mail.openjdk.org/pipermail/serviceability-dev/2024-May/055650.html). > The conversion was done in-place, only renaming it in git. > > I also added a couple of `@since` tags, with only 2 changes I don't want to > split these two fixes into separate PRs. > `CREDENTIALS_FILTER_PATTERN` and `SERIAL_FILTER_PATTERN` were first added in > https://bugs.openjdk.org/browse/JDK-8187556 Nizar Benalla has updated the pull request incrementally with one additional commit since the last revision: fix indentation - Changes: - all: https://git.openjdk.org/jdk/pull/19263/files - new: https://git.openjdk.org/jdk/pull/19263/files/393bf3a3..82536dd9 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19263=04 - incr: https://webrevs.openjdk.org/?repo=jdk=19263=03-04 Stats: 17 lines in 1 file changed: 0 ins; 0 del; 17 mod Patch: https://git.openjdk.org/jdk/pull/19263.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19263/head:pull/19263 PR: https://git.openjdk.org/jdk/pull/19263
Re: RFR: 8332071: Convert package.html files in `java.management.rmi` to package-info.java [v3]
On Mon, 20 May 2024 19:01:47 GMT, Nizar Benalla wrote: >> src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnectorServer.java >> line 121: >> >>> 119: * >>> 120: * @since 10 >>> 121: */ >> >> Please fix the indentation of the "*" for the this comment on >> CREDENTIALS_FILTER_PATTERN so it looks consistent. > > Fixed to be consistent, but the rest of the comment is still indented wrong. > > /** > * Name of the attribute that specifies ... > * (I would have gone the other way and corrected the preceding indentation to the standard). - PR Review Comment: https://git.openjdk.org/jdk/pull/19263#discussion_r1607248724
Re: RFR: 8332071: Convert package.html files in `java.management.rmi` to package-info.java [v3]
On Mon, 20 May 2024 18:52:58 GMT, Roger Riggs wrote: >> Nizar Benalla has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Remove tabs > > src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnectorServer.java > line 121: > >> 119: * >> 120: * @since 10 >> 121: */ > > Please fix the indentation of the "*" for the this comment on > CREDENTIALS_FILTER_PATTERN so it looks consistent. Fixed to be consistent, but the rest of the comment is still indented wrong. /** * Name of the attribute that specifies ... * - PR Review Comment: https://git.openjdk.org/jdk/pull/19263#discussion_r1607157750
Re: RFR: 8332071: Convert package.html files in `java.management.rmi` to package-info.java [v4]
> Please review this change. I converted the `package.html` file to > `package-info.java`, because `javac` cannot recognize `package.html`. > I already brought this up [in the mailing > list](https://mail.openjdk.org/pipermail/serviceability-dev/2024-May/055650.html). > The conversion was done in-place, only renaming it in git. > > I also added a couple of `@since` tags, with only 2 changes I don't want to > split these two fixes into separate PRs. > `CREDENTIALS_FILTER_PATTERN` and `SERIAL_FILTER_PATTERN` were first added in > https://bugs.openjdk.org/browse/JDK-8187556 Nizar Benalla has updated the pull request incrementally with one additional commit since the last revision: suggestions from roger - Changes: - all: https://git.openjdk.org/jdk/pull/19263/files - new: https://git.openjdk.org/jdk/pull/19263/files/232a533a..393bf3a3 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19263=03 - incr: https://webrevs.openjdk.org/?repo=jdk=19263=02-03 Stats: 4 lines in 2 files changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/19263.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19263/head:pull/19263 PR: https://git.openjdk.org/jdk/pull/19263
Re: RFR: 8332071: Convert package.html files in `java.management.rmi` to package-info.java [v3]
On Mon, 20 May 2024 18:50:08 GMT, Roger Riggs wrote: >> Nizar Benalla has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Remove tabs > > src/java.management.rmi/share/classes/javax/management/remote/rmi/package-info.java > line 325: > >> 323: *@since 1.5 >> 324: * >> 325: **/ > > Extra * `*/` is sufficient. Fixed in [393bf3a](https://github.com/openjdk/jdk/pull/19263/commits/393bf3a36e3fb0811f89ef21cbdbb4a32a7a3e21), thanks. - PR Review Comment: https://git.openjdk.org/jdk/pull/19263#discussion_r1607154339
Re: RFR: 8332071: Convert package.html files in `java.management.rmi` to package-info.java [v3]
On Mon, 20 May 2024 18:15:16 GMT, Nizar Benalla wrote: >> Please review this change. I converted the `package.html` file to >> `package-info.java`, because `javac` cannot recognize `package.html`. >> I already brought this up [in the mailing >> list](https://mail.openjdk.org/pipermail/serviceability-dev/2024-May/055650.html). >> The conversion was done in-place, only renaming it in git. >> >> I also added a couple of `@since` tags, with only 2 changes I don't want to >> split these two fixes into separate PRs. >> `CREDENTIALS_FILTER_PATTERN` and `SERIAL_FILTER_PATTERN` were first added in >> https://bugs.openjdk.org/browse/JDK-8187556 > > Nizar Benalla has updated the pull request incrementally with one additional > commit since the last revision: > > Remove tabs src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnectorServer.java line 121: > 119: * > 120: * @since 10 > 121: */ Please fix the indentation of the "*" for the this comment on CREDENTIALS_FILTER_PATTERN so it looks consistent. src/java.management.rmi/share/classes/javax/management/remote/rmi/package-info.java line 325: > 323: *@since 1.5 > 324: * > 325: **/ Extra * `*/` is sufficient. - PR Review Comment: https://git.openjdk.org/jdk/pull/19263#discussion_r1607149187 PR Review Comment: https://git.openjdk.org/jdk/pull/19263#discussion_r1607146703
Re: RFR: 8332071: Convert package.html files in `java.management.rmi` to package-info.java [v3]
> Please review this change. I converted the `package.html` file to > `package-info.java`, because `javac` cannot recognize `package.html`. > I already brought this up [in the mailing > list](https://mail.openjdk.org/pipermail/serviceability-dev/2024-May/055650.html). > The conversion was done in-place, only renaming it in git. > > I also added a couple of `@since` tags, with only 2 changes I don't want to > split these two fixes into separate PRs. > `CREDENTIALS_FILTER_PATTERN` and `SERIAL_FILTER_PATTERN` were first added in > https://bugs.openjdk.org/browse/JDK-8187556 Nizar Benalla has updated the pull request incrementally with one additional commit since the last revision: Remove tabs - Changes: - all: https://git.openjdk.org/jdk/pull/19263/files - new: https://git.openjdk.org/jdk/pull/19263/files/e2471645..232a533a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19263=02 - incr: https://webrevs.openjdk.org/?repo=jdk=19263=01-02 Stats: 30 lines in 1 file changed: 0 ins; 0 del; 30 mod Patch: https://git.openjdk.org/jdk/pull/19263.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19263/head:pull/19263 PR: https://git.openjdk.org/jdk/pull/19263
Re: RFR: 8332071: Convert package.html files in `java.management.rmi` to package-info.java [v3]
On Mon, 20 May 2024 13:19:37 GMT, Daniel Fuchs wrote:
>> Doing that makes git think it's a new file, rather than a rename.
>> I was doing this in
>> [a26ee08](https://github.com/openjdk/jdk/commit/a26ee085b5184d62a879f88f6cca6780e0e4e472)
>> and removed it
>
> LGTM - there are further potential improvements that could be made in this
> file - like replacing `` with `{@code }` and `` with
> `{@snippet }` but I guess that can wait until someone has the inclination and
> bandwidth to do it...
Fixed, found how to "lazily" append to the lines I want so adding the " *"
wasn't an issue
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19263#discussion_r1607108275
Re: RFR: 8332071: Convert package.html files in `java.management.rmi` to package-info.java [v2]
> Please review this change. I converted the `package.html` file to > `package-info.java`, because `javac` cannot recognize `package.html`. > I already brought this up [in the mailing > list](https://mail.openjdk.org/pipermail/serviceability-dev/2024-May/055650.html). > The conversion was done in-place, only renaming it in git. > > I also added a couple of `@since` tags, with only 2 changes I don't want to > split these two fixes into separate PRs. > `CREDENTIALS_FILTER_PATTERN` and `SERIAL_FILTER_PATTERN` were first added in > https://bugs.openjdk.org/browse/JDK-8187556 Nizar Benalla has updated the pull request incrementally with one additional commit since the last revision: append each line with " *" - Changes: - all: https://git.openjdk.org/jdk/pull/19263/files - new: https://git.openjdk.org/jdk/pull/19263/files/52d3d182..e2471645 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19263=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19263=00-01 Stats: 299 lines in 1 file changed: 0 ins; 0 del; 299 mod Patch: https://git.openjdk.org/jdk/pull/19263.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19263/head:pull/19263 PR: https://git.openjdk.org/jdk/pull/19263
Re: RFR: 8332071: Convert package.html files in `java.management.rmi` to package-info.java
On Sun, 19 May 2024 19:05:19 GMT, Nizar Benalla wrote:
>> src/java.management.rmi/share/classes/javax/management/remote/rmi/package-info.java
>> line 26:
>>
>>> 24: */
>>> 25:
>>> 26: /**
>>
>> I assume you'll need to prepend each line with `*` too, which has the side
>> effect of making it appear that every line is changed but I think we just
>> need to get over that.
>
> Doing that makes git think it's a new file, rather than a rename.
> I was doing this in
> [a26ee08](https://github.com/openjdk/jdk/commit/a26ee085b5184d62a879f88f6cca6780e0e4e472)
> and removed it
LGTM - there are further potential improvements that could be made in this file
- like replacing `` with `{@code }` and `` with
`{@snippet }` but I guess that can wait until someone has the inclination and
bandwidth to do it...
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19263#discussion_r1606772388
Re: RFR: 8332071: Convert package.html files in `java.management.rmi` to package-info.java
On Thu, 16 May 2024 10:39:43 GMT, Nizar Benalla wrote: > Please review this change. I converted the `package.html` file to > `package-info.java`, because `javac` cannot recognize `package.html`. > I already brought this up [in the mailing > list](https://mail.openjdk.org/pipermail/serviceability-dev/2024-May/055650.html). > The conversion was done in-place, only renaming it in git. > > I also added a couple of `@since` tags, with only 2 changes I don't want to > split these two fixes into separate PRs. > `CREDENTIALS_FILTER_PATTERN` and `SERIAL_FILTER_PATTERN` were first added in > https://bugs.openjdk.org/browse/JDK-8187556 Looks good - with the asterisk * prefix question from the other comment. I looked to check and yes the since 10 is correct. - Marked as reviewed by kevinw (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19263#pullrequestreview-2065901430
Re: RFR: 8332071: Convert package.html files in `java.management.rmi` to package-info.java
On Sun, 19 May 2024 18:47:50 GMT, Alan Bateman wrote: >> Please review this change. I converted the `package.html` file to >> `package-info.java`, because `javac` cannot recognize `package.html`. >> I already brought this up [in the mailing >> list](https://mail.openjdk.org/pipermail/serviceability-dev/2024-May/055650.html). >> The conversion was done in-place, only renaming it in git. >> >> I also added a couple of `@since` tags, with only 2 changes I don't want to >> split these two fixes into separate PRs. >> `CREDENTIALS_FILTER_PATTERN` and `SERIAL_FILTER_PATTERN` were first added in >> https://bugs.openjdk.org/browse/JDK-8187556 > > src/java.management.rmi/share/classes/javax/management/remote/rmi/package-info.java > line 26: > >> 24: */ >> 25: >> 26: /** > > I assume you'll need to prepend each line with `*` too, which has the side > effect of making it appear that every line is changed but I think we just > need to get over that. Doing that makes git think it's a new file, rather than a rename. I was doing this in [a26ee08](https://github.com/openjdk/jdk/commit/a26ee085b5184d62a879f88f6cca6780e0e4e472) and removed it - PR Review Comment: https://git.openjdk.org/jdk/pull/19263#discussion_r1606086468
Re: RFR: 8332071: Convert package.html files in `java.management.rmi` to package-info.java
On Thu, 16 May 2024 10:39:43 GMT, Nizar Benalla wrote: > Please review this change. I converted the `package.html` file to > `package-info.java`, because `javac` cannot recognize `package.html`. > I already brought this up [in the mailing > list](https://mail.openjdk.org/pipermail/serviceability-dev/2024-May/055650.html). > The conversion was done in-place, only renaming it in git. > > I also added a couple of `@since` tags, with only 2 changes I don't want to > split these two fixes into separate PRs. > `CREDENTIALS_FILTER_PATTERN` and `SERIAL_FILTER_PATTERN` were first added in > https://bugs.openjdk.org/browse/JDK-8187556 src/java.management.rmi/share/classes/javax/management/remote/rmi/package-info.java line 26: > 24: */ > 25: > 26: /** I assume you'll need to prepend each line with `*` too, which has the side effect of making it appear that every line is changed but I think we just need to get over that. - PR Review Comment: https://git.openjdk.org/jdk/pull/19263#discussion_r1606084149
Re: RFR: 8332071: Convert package.html files in `java.management.rmi` to package-info.java
On Thu, 16 May 2024 10:39:43 GMT, Nizar Benalla wrote: > Please review this change. I converted the `package.html` file to > `package-info.java`, because `javac` cannot recognize `package.html`. > I already brought this up [in the mailing > list](https://mail.openjdk.org/pipermail/serviceability-dev/2024-May/055650.html). > The conversion was done in-place, only renaming it in git. > I also added a couple of `@since` tags, with only 2 changes I don't want to > split these two fixes into separate PRs. sorry about the force push, had to. - PR Comment: https://git.openjdk.org/jdk/pull/19263#issuecomment-2119024503
RFR: 8332071: Convert package.html files in `java.management.rmi` to package-info.java
Please review this change. I converted the `package.html` file to `package-info.java`, because `javac` cannot recognize `package.html`. I already brought this up [in the mailing list](https://mail.openjdk.org/pipermail/serviceability-dev/2024-May/055650.html). The conversion was done in-place, only renaming it in git. I also added a couple of `@since` tags, with only 2 changes I don't want to split these two fixes into separate PRs. - Commit messages: - remove whitespace - - convert package.html to package-info.java Changes: https://git.openjdk.org/jdk/pull/19263/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19263=00 Issue: https://bugs.openjdk.org/browse/JDK-8332071 Stats: 37 lines in 2 files changed: 4 ins; 3 del; 30 mod Patch: https://git.openjdk.org/jdk/pull/19263.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19263/head:pull/19263 PR: https://git.openjdk.org/jdk/pull/19263
