epugh commented on code in PR #2577:
URL: https://github.com/apache/solr/pull/2577#discussion_r1685667779
##########
solr/bin/solr.cmd:
##########
@@ -1525,10 +1524,10 @@ IF "!ZK_OP!"=="" (
set CONNECTION_PARAMS=""
IF "!ZK_OP!"=="" (
- set CONNECTION_PARAMS="-solrUrl !ZK_SOLR_URL!"
+ set CONNECTION_PARAMS="--solr-url !ZK_SOLR_URL!"
)
ELSE (
- set CONNECTION_PARAMS="-zkHost ZK_HOST!"
+ set CONNECTION_PARAMS="--zk-host ZK_HOST!"
Review Comment:
yep! similar to above issue with `-solrUrl`
##########
solr/bin/solr.cmd:
##########
@@ -314,7 +313,7 @@ goto done
@echo -f Start Solr in foreground; default starts Solr in the
background
@echo and sends stdout / stderr to solr-PORT-console.log
@echo.
-@echo -c or -cloud Start Solr in SolrCloud mode; if -z not supplied and
ZK_HOST not defined in
+@echo -c or --cloud Start Solr in SolrCloud mode; if -z not supplied and
ZK_HOST not defined in
Review Comment:
correct! `--cloud` is what the docs should say. We need to preserve the
use of `-cloud` in 9x however...
##########
solr/bin/solr.cmd:
##########
@@ -1525,10 +1524,10 @@ IF "!ZK_OP!"=="" (
set CONNECTION_PARAMS=""
IF "!ZK_OP!"=="" (
- set CONNECTION_PARAMS="-solrUrl !ZK_SOLR_URL!"
+ set CONNECTION_PARAMS="--solr-url !ZK_SOLR_URL!"
Review Comment:
good catch! I suspect because we still support `-solrUrl` that is why this
works...
##########
solr/bin/solr.cmd:
##########
@@ -292,7 +292,6 @@ IF "%FIRST_ARG%"=="-help" goto run_solrcli
IF "%FIRST_ARG%"=="-usage" goto run_solrcli
IF "%FIRST_ARG%"=="-h" goto run_solrcli
IF "%FIRST_ARG%"=="--help" goto run_solrcli
-IF "%FIRST_ARG%"=="-help" goto run_solrcli
Review Comment:
`-help` is a special case (and we shoild probably comment that it is so) in
the idea that folks might do -h, --help, and -help equally when trying to get
to the help!
##########
solr/packaging/test/test_adminconsole_urls.bats:
##########
@@ -25,7 +25,7 @@ teardown() {
# save a snapshot of SOLR_HOME for failed tests
save_home_on_failure
- solr stop -all >/dev/null 2>&1
+ solr stop --all >/dev/null 2>&1
Review Comment:
I honestly stayed away from touching `bin/solr start` and `bin/solr stop`
since I suspect lots of folks have scripts that call those and otherwise it
seemed complicated. I am up for changing `-all` to `--all`, however, can we
augment the bats tests to try both? Just to verify that `-all` and `-all` both
work?
##########
solr/core/src/test/org/apache/solr/cli/AuthToolTest.java:
##########
@@ -70,7 +70,7 @@ public void testEnableAuth() throws Exception {
dir.toAbsolutePath().toString(),
"--solr-include-file",
solrIncludeFile.toAbsolutePath().toString(),
- "-credentials",
Review Comment:
so, @gerlowskija and I had a conversation that we ideally should be
randomizing some of these, like `-credentials` and `--credentials` to make sure
things continue to work with deprecated and non deprecated options. I took
the short cut of just randomly picking between the two, however maybe we should
introduce a method here to handle this? Alternatively, maybe a comment on why
`-credentials` just to remind the next person?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
