malliaridis commented on code in PR #2721:
URL: https://github.com/apache/solr/pull/2721#discussion_r1772845049
##########
solr/core/src/java/org/apache/solr/cli/SolrCLI.java:
##########
@@ -148,9 +154,21 @@ public class SolrCLI implements CLIO {
public static final Option OPTION_VERBOSE =
Option.builder("v")
.longOpt("verbose")
+ .deprecated(
+ DeprecatedAttributes.builder()
+ .setForRemoval(true)
+ .setSince("9.8")
+ .setDescription("Use --debug instead")
+ .get())
.required(false)
.desc("Enable verbose command output.")
.build();
+ public static final Option OPTION_DEBUG =
+ Option.builder("d")
+ .longOpt("debug")
+ .required(false)
+ .desc("Enable additional command output.")
+ .build();
Review Comment:
I wasn't sure if we would go for the `--debug` option. Shall we also
deprecate the `-d` for all the other arguments as well in this PR? We use `-d`
in quite a few places and providing the `-d` now for `debug` to all tools may
cause unexpected behavior.
##########
solr/core/src/java/org/apache/solr/cli/SolrCLI.java:
##########
@@ -148,9 +154,21 @@ public class SolrCLI implements CLIO {
public static final Option OPTION_VERBOSE =
Review Comment:
Do we mark deprecated options with `@Deprecated`? If so, this would be
beneficial for `OPTION_VERBOSE`.
##########
solr/core/src/java/org/apache/solr/cli/SolrCLI.java:
##########
@@ -137,6 +137,12 @@ public class SolrCLI implements CLIO {
public static final Option OPTION_SOLRURL =
Option.builder("url")
.longOpt("solr-url")
+ .deprecated(
+ DeprecatedAttributes.builder()
+ .setForRemoval(true)
+ .setSince("9.7")
+ .setDescription("Use --solr-url instead")
+ .get())
Review Comment:
Leaving this as a note: Splitting `url` from `solr-url` would have a way too
large impact in the rest of the tools, so simply marking both as deprecated is
a good tradeoff.
##########
solr/core/src/java/org/apache/solr/cli/SolrCLI.java:
##########
@@ -308,7 +326,8 @@ protected static void checkSslStoreSysProp(String
solrInstallDir, String key) {
}
public static void raiseLogLevelUnlessVerbose(CommandLine cli) {
- if (!cli.hasOption(OPTION_VERBOSE.getOpt())) {
+ if (!cli.hasOption((SolrCLI.OPTION_DEBUG.getOpt()))
+ || cli.hasOption(SolrCLI.OPTION_VERBOSE.getOpt())) {
Review Comment:
Have you considered putting both options (verbose and debug) into an
`OptionGroup` (mutually exclusive)? This would allow single option (group)
checks.
--
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: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
