Claudenw commented on code in PR #268:
URL: https://github.com/apache/creadur-rat/pull/268#discussion_r1676800742
##########
apache-rat-core/src/main/java/org/apache/rat/DeprecationReporter.java:
##########
@@ -83,18 +83,6 @@ public static void logDeprecated(final Class<?> clazz) {
}
}
- /**
- * Log deprecated options
- * @param log The log to write to.
- * @param option potentially deprecated option to log.
- */
- // TODO remove this when commons-cli 1.7.1 or higher is available
- public static void logDeprecated(final Log log, final Option option) {
- if (option.isDeprecated()) {
- getLogReporter(log).accept(option);
- }
- }
-
Review Comment:
@ottlinger
commons-cli implemented deprecated options in 1.7.0 but there was a bug with
it not reporting usage if the string was not used to lookup the usage. 1.8.0
solved that problem.
While we were using 1.7.0 there was code to explicitly check if the options
were deprecated. With commons-cli 1.8.0 this this is no longer needed and is
moved.
##########
apache-rat-core/src/test/java/org/apache/rat/OptionCollectionTest.java:
##########
@@ -118,27 +121,44 @@ public void testDeprecatedUseLogged() throws IOException {
TestingLog log = new TestingLog();
try {
DefaultLog.setInstance(log);
- String[] args = {longOpt(OptionCollection.DIR), "foo", "-a"};
- ReportConfiguration config = OptionCollection.parseCommands(args,
(o) -> {
- }, true);
-
+ String[] args = {longOpt(OptionCollection.DIR), "target", "-a"};
+ ReportConfiguration config = OptionCollection.parseCommands(args,
o -> fail("Help printed"), true);
} finally {
DefaultLog.setInstance(null);
}
log.assertContains("WARN: Option [-d, --dir] used. Deprecated for
removal since 0.17: Use '--'");
+ log.assertNotContains("WARN: Option [-d, --dir] used. Deprecated for
removal since 0.17: Use '--'", 1);
log.assertContains("WARN: Option [-a] used. Deprecated for removal
since 0.17: Use '-A' or '--addLicense'");
+ log.assertNotContains("WARN: Option [-a] used. Deprecated for removal
since 0.17: Use '-A' or '--addLicense'", 1);
+ }
+
+ @Test
+ public void testDirOptionCapturesDirectoryToScan() throws IOException {
+ TestingLog log = new TestingLog();
+ ReportConfiguration config = null;
+ try {
+ DefaultLog.setInstance(log);
+ String[] args = {longOpt(OptionCollection.DIR), "foo"};
+ config = OptionCollection.parseCommands(args, (o) -> {
+ }, true);
+ } finally {
+ DefaultLog.setInstance(null);
+ }
+ assertThat(config).isNotNull();
+ log.assertContains("WARN: Option [-d, --dir] used. Deprecated for
removal since 0.17: Use '--'");
+ log.assertNotContains("WARN: Option [-d, --dir] used. Deprecated for
removal since 0.17: Use '--'", 1);
Review Comment:
Good catch. I have reworked the test and the utility classes to have
methods
- contains( expected )
- containsExactly( times, expected )
- notContains( unexpected )
##########
apache-rat-core/src/test/java/org/apache/rat/testhelpers/TextUtils.java:
##########
@@ -78,4 +94,23 @@ public static void assertNotContains(final String find,
final String target) {
assertFalse(
target.contains(find), () -> "Target does contain the text: "
+ find + "\n" + target);
}
+
Review Comment:
done
--
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.
To unsubscribe, e-mail: dev-unsubscr...@creadur.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
