Re: RFR: JDK-8286604: Update InputStream and OutputStream to use @implSpec [v2]
On Thu, 12 May 2022 20:04:33 GMT, Joe Darcy wrote: >> While doing a CSR review of another issue, I noticed some cases in >> InputStream and OutputStream what would benefit from being upgraded to >> implSpec and related javadoc tags. >> >> The "A subclass must provide an implementation of this method." statements >> on several abstract methods don't add much value, but I chose to leave them >> in for this request. >> >> Please also review the corresponding CSR: >> https://bugs.openjdk.java.net/browse/JDK-8286605 > > Joe Darcy has updated the pull request incrementally with one additional > commit since the last revision: > > Respond to review feedback. Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8663
Re: RFR: JDK-8286604: Update InputStream and OutputStream to use @implSpec [v2]
On Thu, 12 May 2022 20:04:33 GMT, Joe Darcy wrote: >> While doing a CSR review of another issue, I noticed some cases in >> InputStream and OutputStream what would benefit from being upgraded to >> implSpec and related javadoc tags. >> >> The "A subclass must provide an implementation of this method." statements >> on several abstract methods don't add much value, but I chose to leave them >> in for this request. >> >> Please also review the corresponding CSR: >> https://bugs.openjdk.java.net/browse/JDK-8286605 > > Joe Darcy has updated the pull request incrementally with one additional > commit since the last revision: > > Respond to review feedback. CSR also updated to reflect changes made in response to review comments; thanks. - PR: https://git.openjdk.java.net/jdk/pull/8663
Re: RFR: JDK-8286604: Update InputStream and OutputStream to use @implSpec [v2]
On Wed, 11 May 2022 22:43:43 GMT, liach wrote:
>> Joe Darcy has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Respond to review feedback.
>
> src/java.base/share/classes/java/io/OutputStream.java line 151:
>
>> 149: * The {@code write} method of {@code OutputStream} calls
>> 150: * the write method of one argument on each of the bytes to be
>> 151: * written out. Subclasses are encouraged to override this method
>> and
>
> Shouldn't the "subclasses" information be part of the API Note?
Sure; moved to an apiNote.
-
PR: https://git.openjdk.java.net/jdk/pull/8663
Re: RFR: JDK-8286604: Update InputStream and OutputStream to use @implSpec [v2]
> While doing a CSR review of another issue, I noticed some cases in > InputStream and OutputStream what would benefit from being upgraded to > implSpec and related javadoc tags. > > The "A subclass must provide an implementation of this method." statements on > several abstract methods don't add much value, but I chose to leave them in > for this request. > > Please also review the corresponding CSR: > https://bugs.openjdk.java.net/browse/JDK-8286605 Joe Darcy has updated the pull request incrementally with one additional commit since the last revision: Respond to review feedback. - Changes: - all: https://git.openjdk.java.net/jdk/pull/8663/files - new: https://git.openjdk.java.net/jdk/pull/8663/files/a29e5c36..7d88f44c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=8663&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8663&range=00-01 Stats: 13 lines in 2 files changed: 3 ins; 8 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/8663.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8663/head:pull/8663 PR: https://git.openjdk.java.net/jdk/pull/8663
Re: RFR: JDK-8286604: Update InputStream and OutputStream to use @implSpec
On Thu, 12 May 2022 12:24:17 GMT, Alan Bateman wrote:
>> While doing a CSR review of another issue, I noticed some cases in
>> InputStream and OutputStream what would benefit from being upgraded to
>> implSpec and related javadoc tags.
>>
>> The "A subclass must provide an implementation of this method." statements
>> on several abstract methods don't add much value, but I chose to leave them
>> in for this request.
>>
>> Please also review the corresponding CSR:
>> https://bugs.openjdk.java.net/browse/JDK-8286605
>
> src/java.base/share/classes/java/io/InputStream.java line 177:
>
>> 175: *
>> 176: * @apiNote
>> 177: * A subclass must provide an implementation of this method.
>
> Is this sentence useful to keep? The method is abstract so a concrete
> implementation has to implement it. On the other other hand, an abstract
> subclass does not need to implement it.
If such a sentence occurred in new code, I would recommend it be removed. I
left it in place in the spirit of just adding apiNote, implSpec, etc., but I'm
happy to delete these comments too. I assume it was deemed useful to readers of
JDK 1.0, but the assumed background of Java developers now is rather different
:-)
> src/java.base/share/classes/java/io/InputStream.java line 688:
>
>> 686: * @implSpec
>> 687: * The {@code mark} method of {@code InputStream} does
>> 688: * nothing.
>
> Minor nit but the line break can be removed so that "nothing" is on the same
> line.
Sure. (I default to not making such reflow changes in the initial version of a
patch to avoid spurious diffs.
-
PR: https://git.openjdk.java.net/jdk/pull/8663
Re: RFR: JDK-8286604: Update InputStream and OutputStream to use @implSpec
On Wed, 11 May 2022 20:40:30 GMT, Joe Darcy wrote:
> While doing a CSR review of another issue, I noticed some cases in
> InputStream and OutputStream what would benefit from being upgraded to
> implSpec and related javadoc tags.
>
> The "A subclass must provide an implementation of this method." statements on
> several abstract methods don't add much value, but I chose to leave them in
> for this request.
>
> Please also review the corresponding CSR:
> https://bugs.openjdk.java.net/browse/JDK-8286605
src/java.base/share/classes/java/io/InputStream.java line 177:
> 175: *
> 176: * @apiNote
> 177: * A subclass must provide an implementation of this method.
Is this sentence useful to keep? The method is abstract so a concrete
implementation has to implement it. On the other other hand, an abstract
subclass does not need to implement it.
src/java.base/share/classes/java/io/InputStream.java line 688:
> 686: * @implSpec
> 687: * The {@code mark} method of {@code InputStream} does
> 688: * nothing.
Minor nit but the line break can be removed so that "nothing" is on the same
line.
-
PR: https://git.openjdk.java.net/jdk/pull/8663
Re: RFR: JDK-8286604: Update InputStream and OutputStream to use @implSpec
On Wed, 11 May 2022 20:40:30 GMT, Joe Darcy wrote:
> While doing a CSR review of another issue, I noticed some cases in
> InputStream and OutputStream what would benefit from being upgraded to
> implSpec and related javadoc tags.
>
> The "A subclass must provide an implementation of this method." statements on
> several abstract methods don't add much value, but I chose to leave them in
> for this request.
>
> Please also review the corresponding CSR:
> https://bugs.openjdk.java.net/browse/JDK-8286605
src/java.base/share/classes/java/io/OutputStream.java line 151:
> 149: * The {@code write} method of {@code OutputStream} calls
> 150: * the write method of one argument on each of the bytes to be
> 151: * written out. Subclasses are encouraged to override this method and
Shouldn't the "subclasses" information be part of the API Note?
-
PR: https://git.openjdk.java.net/jdk/pull/8663
Re: RFR: JDK-8286604: Update InputStream and OutputStream to use @implSpec
On Wed, 11 May 2022 20:40:30 GMT, Joe Darcy wrote: > While doing a CSR review of another issue, I noticed some cases in > InputStream and OutputStream what would benefit from being upgraded to > implSpec and related javadoc tags. > > The "A subclass must provide an implementation of this method." statements on > several abstract methods don't add much value, but I chose to leave them in > for this request. > > Please also review the corresponding CSR: > https://bugs.openjdk.java.net/browse/JDK-8286605 Associated CSR also reviewed. - Marked as reviewed by iris (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8663
Re: RFR: JDK-8286604: Update InputStream and OutputStream to use @implSpec
On Wed, 11 May 2022 20:40:30 GMT, Joe Darcy wrote: > While doing a CSR review of another issue, I noticed some cases in > InputStream and OutputStream what would benefit from being upgraded to > implSpec and related javadoc tags. > > The "A subclass must provide an implementation of this method." statements on > several abstract methods don't add much value, but I chose to leave them in > for this request. > > Please also review the corresponding CSR: > https://bugs.openjdk.java.net/browse/JDK-8286605 Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8663
Re: RFR: JDK-8286604: Update InputStream and OutputStream to use @implSpec
On Wed, 11 May 2022 20:40:30 GMT, Joe Darcy wrote: > While doing a CSR review of another issue, I noticed some cases in > InputStream and OutputStream what would benefit from being upgraded to > implSpec and related javadoc tags. > > The "A subclass must provide an implementation of this method." statements on > several abstract methods don't add much value, but I chose to leave them in > for this request. > > Please also review the corresponding CSR: > https://bugs.openjdk.java.net/browse/JDK-8286605 Looks fine. - Marked as reviewed by bpb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8663
RFR: JDK-8286604: Update InputStream and OutputStream to use @implSpec
While doing a CSR review of another issue, I noticed some cases in InputStream and OutputStream what would benefit from being upgraded to implSpec and related javadoc tags. The "A subclass must provide an implementation of this method." statements on several abstract methods don't add much value, but I chose to leave them in for this request. Please also review the corresponding CSR: https://bugs.openjdk.java.net/browse/JDK-8286605 - Commit messages: - JDK-8286604: Update InputStream and OutputStream to use @implSpec Changes: https://git.openjdk.java.net/jdk/pull/8663/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=8663&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8286604 Stats: 36 lines in 2 files changed: 18 ins; 4 del; 14 mod Patch: https://git.openjdk.java.net/jdk/pull/8663.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8663/head:pull/8663 PR: https://git.openjdk.java.net/jdk/pull/8663
