Re: RFR: JDK-8286783: Expand use of @inheritDoc in InputStream and OutputStream subclasses [v4]
On Mon, 16 May 2022 17:53:17 GMT, Alan Bateman wrote: > thanks for the updates. Updated the CSR to match this version. - PR: https://git.openjdk.java.net/jdk/pull/8717
Re: RFR: JDK-8286783: Expand use of @inheritDoc in InputStream and OutputStream subclasses [v4]
On Mon, 16 May 2022 17:29:16 GMT, Joe Darcy wrote: >> Make the javadoc in the InputStream and OutputStream subclasses in core libs >> DRY-er by use of inheritDoc. (Any analagous changes to AudioInputStream in >> client libs will be done another a separate bug.) When the time comes, will >> do any necessary merging for the changes in[JDK-8286200. >> >> Please also review the CSR to cover the introduction of implspec and >> implNote tags: https://bugs.openjdk.java.net/browse/JDK-8286784 > > Joe Darcy has updated the pull request incrementally with one additional > commit since the last revision: > > Add @Override's requested in review feedback. thanks for the updates. - Marked as reviewed by alanb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8717
Re: RFR: JDK-8286783: Expand use of @inheritDoc in InputStream and OutputStream subclasses [v4]
> Make the javadoc in the InputStream and OutputStream subclasses in core libs > DRY-er by use of inheritDoc. (Any analagous changes to AudioInputStream in > client libs will be done another a separate bug.) When the time comes, will > do any necessary merging for the changes in[JDK-8286200. > > Please also review the CSR to cover the introduction of implspec and implNote > tags: https://bugs.openjdk.java.net/browse/JDK-8286784 Joe Darcy has updated the pull request incrementally with one additional commit since the last revision: Add @Override's requested in review feedback. - Changes: - all: https://git.openjdk.java.net/jdk/pull/8717/files - new: https://git.openjdk.java.net/jdk/pull/8717/files/07e8a6a6..c31a408d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8717=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8717=02-03 Stats: 11 lines in 5 files changed: 11 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/8717.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8717/head:pull/8717 PR: https://git.openjdk.java.net/jdk/pull/8717
Re: RFR: JDK-8286783: Expand use of @inheritDoc in InputStream and OutputStream subclasses [v3]
On Mon, 16 May 2022 16:56:00 GMT, Joe Darcy wrote: > I added the `@Override` annotations to methods I reviewed and/or updated; it > was not necessarily an exhaustive process. Yeah, there are inconsistencies as a result and I think we should go the extra mile and add to the methods such as readAllBbytes, readNBytes that weren't changed. - PR: https://git.openjdk.java.net/jdk/pull/8717
Re: RFR: JDK-8286783: Expand use of @inheritDoc in InputStream and OutputStream subclasses [v3]
On Sun, 15 May 2022 21:16:53 GMT, Pavel Rappo wrote:
> 1. One the one hand, it's not clear to me what criterion was used for adding
> `@Override` annotations. On the other hand, the more `@Override` annotations
> a codebase has, the better.
>
> 2. Have you compared the resulting documentation before and after the
> change? Aside from added `@implSpec` and `@implNote`, were there anything
> anything different?
>
> 3. I wonder if it makes sense to also reduce the size of the doc comments
> by changing explicit documentation inheritance for the `@param` and `@return`
> tags to implicit one, i.e. removing the tags on the overrider's side.
I added the `@Override` annotations to methods I reviewed and/or updated; it
was not necessarily an exhaustive process. I have not run specdiff to
double-check the expected changes; it would be more thorough to do so. The
intention is to only have minor wording changes apart from the `@implSpec` and
`@implNote` tags. Stylistically, I'd prefer the explicit `@foo {@inhertiDoc}`
idiom to indicate the code just want to use the javadoc from a supertype.
-
PR: https://git.openjdk.java.net/jdk/pull/8717
Re: RFR: JDK-8286783: Expand use of @inheritDoc in InputStream and OutputStream subclasses [v3]
> Make the javadoc in the InputStream and OutputStream subclasses in core libs > DRY-er by use of inheritDoc. (Any analagous changes to AudioInputStream in > client libs will be done another a separate bug.) When the time comes, will > do any necessary merging for the changes in[JDK-8286200. > > Please also review the CSR to cover the introduction of implspec and implNote > tags: https://bugs.openjdk.java.net/browse/JDK-8286784 Joe Darcy has updated the pull request incrementally with one additional commit since the last revision: Update copyright, change whitespace. - Changes: - all: https://git.openjdk.java.net/jdk/pull/8717/files - new: https://git.openjdk.java.net/jdk/pull/8717/files/c84ef79a..07e8a6a6 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8717=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8717=01-02 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/8717.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8717/head:pull/8717 PR: https://git.openjdk.java.net/jdk/pull/8717
Re: RFR: JDK-8286783: Expand use of @inheritDoc in InputStream and OutputStream subclasses [v2]
> Make the javadoc in the InputStream and OutputStream subclasses in core libs > DRY-er by use of inheritDoc. (Any analagous changes to AudioInputStream in > client libs will be done another a separate bug.) When the time comes, will > do any necessary merging for the changes in[JDK-8286200. > > Please also review the CSR to cover the introduction of implspec and implNote > tags: https://bugs.openjdk.java.net/browse/JDK-8286784 Joe Darcy has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains two commits: - Merge branch 'master' into JDK-8286783 - JDK-8286783: Expand use of @inheritDoc in InputStream and OutputStream subclasses - Changes: https://git.openjdk.java.net/jdk/pull/8717/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8717=01 Stats: 189 lines in 13 files changed: 49 ins; 45 del; 95 mod Patch: https://git.openjdk.java.net/jdk/pull/8717.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8717/head:pull/8717 PR: https://git.openjdk.java.net/jdk/pull/8717
Re: RFR: JDK-8286783: Expand use of @inheritDoc in InputStream and OutputStream subclasses
On Sun, 15 May 2022 18:36:07 GMT, Joe Darcy wrote:
> Make the javadoc in the InputStream and OutputStream subclasses in core libs
> DRY-er by use of inheritDoc. (Any analagous changes to AudioInputStream in
> client libs will be done another a separate bug.) When the time comes, will
> do any necessary merging for the changes in[JDK-8286200.
>
> Please also review the CSR to cover the introduction of implspec and implNote
> tags: https://bugs.openjdk.java.net/browse/JDK-8286784
1. One the one hand, it's not clear to me what criterion was used for adding
`@Override` annotations. On the other hand, the more `@Override` annotations a
codebase has, the better.
2. Have you compared the resulting documentation before and after the change?
Aside from added `@implSpec` and `@implNote`, were there anything anything
different?
3. I wonder if it makes sense to also reduce the size of the doc comments by
changing explicit documentation inheritance for the `@param` and `@return` tags
to implicit one, i.e. removing the tags on the overrider's side.
src/java.base/share/classes/java/io/SequenceInputStream.java line 217:
> 215: * before the {@code close} method returns.
> 216: *
> 217: * @throws IOException {@inheritDoc}
Unexpected re-indentation; other similar cases do not have it.
-
PR: https://git.openjdk.java.net/jdk/pull/8717
Re: RFR: JDK-8286783: Expand use of @inheritDoc in InputStream and OutputStream subclasses
On Sun, 15 May 2022 18:36:07 GMT, Joe Darcy wrote: > Make the javadoc in the InputStream and OutputStream subclasses in core libs > DRY-er by use of inheritDoc. (Any analagous changes to AudioInputStream in > client libs will be done another a separate bug.) When the time comes, will > do any necessary merging for the changes in[JDK-8286200. > > Please also review the CSR to cover the introduction of implspec and implNote > tags: https://bugs.openjdk.java.net/browse/JDK-8286784 Picking FileInputStream at random, I wonder if the `@inheritDoc` on transferTo is needed. Also, I think you've attempted to add `@Override` to all overridden methods but some may have been missed (e.g. readAllBytes and readNBytes). - PR: https://git.openjdk.java.net/jdk/pull/8717
RFR: JDK-8286783: Expand use of @inheritDoc in InputStream and OutputStream subclasses
Make the javadoc in the InputStream and OutputStream subclasses in core libs DRY-er by use of inheritDoc. (Any analagous changes to AudioInputStream in client libs will be done another a separate bug.) When the time comes, will do any necessary merging for the changes in[JDK-8286200. Please also review the CSR to cover the introduction of implspec and implNote tags: https://bugs.openjdk.java.net/browse/JDK-8286784 - Commit messages: - JDK-8286783: Expand use of @inheritDoc in InputStream and OutputStream subclasses Changes: https://git.openjdk.java.net/jdk/pull/8717/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8717=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8286783 Stats: 190 lines in 13 files changed: 49 ins; 45 del; 96 mod Patch: https://git.openjdk.java.net/jdk/pull/8717.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8717/head:pull/8717 PR: https://git.openjdk.java.net/jdk/pull/8717
