NightOwl888 commented on code in PR #961: URL: https://github.com/apache/lucenenet/pull/961#discussion_r1772456674
########## src/dotnet/tools/lucene-cli/docs/index.md: ########## @@ -11,7 +11,7 @@ The Lucene.NET command line interface (CLI) is a new cross-platform toolchain wi Perform a one-time install of the lucene-cli tool using the following dotnet CLI command: ```console -dotnet tool install lucene-cli -g --version 4.8.0-beta00016 +dotnet tool install lucene-cli -g --version EnvVar:LuceneNetVersion Review Comment: This is a good suggestion, but while it works *right now*, it does not really address all of the future concerns of having multiple release versions in the wild. Is it convenient to install the latest version with `--prerelease`? Yes. Is that what users should do most of the time? Probably not. They will get into trouble if they try to use a version of the tool that doesn't match the version of Lucene.NET in their project. This is where we need to be vigilant because in Java you can simply run the commands on the same package that you reference in the project, so the version will always match. But in our case, we need to ensure to lead the users into installing the *matching* version, not the *latest* version. It would be very bad if someone runs the [lucene index fix](https://github.com/apache/lucenenet/blob/cb9545fcde401dea9fdafabf3d99a3fb539f25e9/src/dotnet/tools/lucene-cli/docs/index/fix.md) command using a version that doesn't match what their project is using and they don't have a backup. Although I think there are checks for most of these commands to prevent them from running on non-matching versions, that doesn't cover any potential binary changes that we made to the codecs to address bugs while in prerelease. After giving it more consideration, I think we should make the following improvements: 1. Change the text from "Perform a one-time install of the lucene-cli tool using the following dotnet CLI command:" to "Perform a one-time install of the lucene-cli tool using the [dotnet tool install](https://learn.microsoft.com/en-us/dotnet/core/tools/dotnet-tool-install) command, which will install the specified version:" 2. Change the note below the command to read "The version of the CLI you install must match the version of Lucene.NET you use. The version can be specified using the [`--version` option](https://learn.microsoft.com/en-us/dotnet/core/tools/dotnet-tool-install#options) of the [dotnet tool install](https://learn.microsoft.com/en-us/dotnet/core/tools/dotnet-tool-install) command. For a list of available versions, see the [versions tab of the lucene-cli NuGet package](https://www.nuget.org/packages/lucene-cli/4.8.0-beta00016#versions-body-tab) or run the [dotnet tool list](https://learn.microsoft.com/en-us/dotnet/core/tools/dotnet-tool-list) command using the package id `lucene-cli`." Maybe we should even go further and show examples of how to run the dotnet tool list command inside of that note. As for whether to keep the `--prerelease` option in the documentation (at least until we are out of prerelease) or specify the `--version`, I think the `--version` is the more conservative approach that will help to prevent users from getting it wrong. -- 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]
