jaykay12 commented on code in PR #5190:
URL: https://github.com/apache/solr/pull/5190#discussion_r4345780534
##########
.github/workflows/bin-solr-test.yml:
##########
@@ -30,8 +30,7 @@ jobs:
- name: Test the bin/solr script
run: ./gradlew integrationTests
- # Reuses the distribution built above, so this only costs the curated
suite run.
- - name: Test the bin/solr script with the experimental picocli CLI
+ - name: Test the bin/solr script with PicoCLI
Review Comment:
when removing the word: "experimental" rephrased this as well & removed the
comment.
##########
dev-docs/ref-guide/antora.adoc:
##########
@@ -81,22 +81,11 @@ Note that upgrading picocli can change the generated
wording without any annotat
=== Adding docs for a new tool
-The new command will automatically appear in the generated pages, in the
navigation and in the command table on `pages/cli/index.adoc` when running
`gradlew generateCliDocs`.
+The new command will automatically appear in the generated pages, in the
navigation and in the command table on `pages/cli/index.adoc` when running
`./gradlew generateCliDocs`.
`pages/cli/index.adoc` is the only hand-written page in that directory;
everything matching `solr-*.adoc` is generated and must not be edited by hand.
`solr-control-script-reference.adoc` documents the default (commons-cli)
implementation and should not gain per-command links to the generated pages
while the picocli implementation is experimental.
-=== What a tool migration should include
-
-When migrating a tool to picocli:
-
-* A `*PicocliTest` subclass of the tool's existing test that overrides only
the invocation strategy, so every case runs on both implementations. See
`ZkSubcommandsPicocliTest` for the pattern.
-* The tool's `.bats` file added to `integrationTestsPicocli` in
`solr/packaging/build.gradle`, but only once *every* command that suite invokes
has been migrated, not just the one under test.
- `test_auth.bats`, for example, uses `solr assert` to wait for startup, so it
cannot be added until the assert command is migrated.
- Be aware that a failure part-way through that suite is especially awkward:
`solr auth enable` appends `SOLR_AUTH_TYPE` and `SOLR_AUTHENTICATION_OPTS` to
the `solr.in.sh` of the *built distribution* under `solr/packaging/build/`, so
an aborted run leaves every later `bin/solr` invocation trying to authenticate.
- If BATS runs start hanging for no apparent reason, check that file, and
delete the distribution directory to force `installFullDist` to recreate it.
-* A `gradlew :solr:solr-ref-guide:generateCliDocs` run, with the result
committed.
-* The tool registered in `SolrCLI`'s `subcommands` only once `callTool()`
actually works; `SolrCLIPicocliTest` enforces the rules that apply to every
registered command.
Review Comment:
Since, all tools migrations are done. these instructions in the ref-guides
are obselete now.
let me know still if we would want to keep them, will restore back if
reviewers feel otherwise.
##########
solr/solr-ref-guide/build.gradle:
##########
@@ -592,15 +592,6 @@ def ASF_LICENSE_HEADER = """\
// under the License.
"""
-def EXPERIMENTAL_NOTICE = """\
-[WARNING]
-====
-This page documents the *experimental* picocli implementation of `bin/solr`,
which is not enabled by default.
-For what `bin/solr` does by default, see
xref:solr-control-script-reference.adoc[].
-See xref:cli/index.adoc[] to enable this implementation and for its known
limitations.
-====
-"""
-
Review Comment:
This notice removal, has made this PR (number of files changed sizes to
large), almost 42 adocs in the CLI are getting changed.
lmk, if this should get merged separately, will pull this out in a follow-up
PR, if size of PR is a concern,
--
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]
