garydgregory commented on a change in pull request #173:
URL: https://github.com/apache/commons-io/pull/173#discussion_r538433221
##########
File path: src/main/java/org/apache/commons/io/input/CloseShieldInputStream.java
##########
@@ -50,4 +54,16 @@ public void close() {
in = ClosedInputStream.CLOSED_INPUT_STREAM;
}
+ /**
+ * Creates a proxy that shields the given input stream from being
+ * closed.
+ *
+ * @param in underlying input stream
Review comment:
`@param in underlying input stream` -> `@param inputStream the input
stream to wrap`
##########
File path: src/main/java/org/apache/commons/io/input/CloseShieldReader.java
##########
@@ -50,4 +54,16 @@ public void close() {
in = ClosedReader.CLOSED_READER;
}
+ /**
+ * Creates a proxy that shields the given reader from being
+ * closed.
+ *
+ * @param in underlying reader
+ * @return the created proxy
+ * @since 2.9.0
+ */
+ public static CloseShieldReader wrap(final Reader in) {
Review comment:
as see other Javadoc comments, in this case `in` -> `reader`
##########
File path: src/main/java/org/apache/commons/io/input/CloseShieldInputStream.java
##########
@@ -35,7 +35,11 @@
* closed.
*
* @param in underlying input stream
+ * @deprecated Using this constructor prevents IDEs from warning if
+ * the underlying input stream is never closed.
+ * Use {@link #wrap(InputStream)} instead.
*/
+ @Deprecated
Review comment:
I don't agree with deprecating these APIs, different versions of
different IDEs raise different warnings depending on your preferred
configuration of said IDE. Let's keep this PR simpler and lighter, we can
always add deprecation later if it is truly warranted.
##########
File path: src/test/java/org/apache/commons/io/output/CloseShieldWriterTest.java
##########
@@ -40,19 +40,14 @@
@BeforeEach
public void setUp() {
original = spy(new StringBuilderWriter());
- shielded = new CloseShieldWriter(original);
+ shielded = CloseShieldWriter.wrap(original);
}
@Test
public void testClose() throws IOException {
shielded.close();
verify(original, never()).close();
- try {
- shielded.write('x');
- fail("write(c)");
- } catch (final IOException ignore) {
- // expected
- }
+ assertThrows(IOException.class, () -> shielded.write('x'), "write(c)");
Review comment:
Nice use of a new JUnit 5 API! 👍
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
