NightOwl888 commented on code in PR #963:
URL: https://github.com/apache/lucenenet/pull/963#discussion_r1776118867
##########
src/dotnet/tools/lucene-cli/commands/benchmark/benchmark-find-quality-queries/BenchmarkFindQualityQueriesCommand.cs:
##########
@@ -25,7 +26,7 @@ public class Configuration : ConfigurationBase
{
public Configuration(CommandLineOptions options)
{
- this.Main = (args) => QueryDriver.Main(args);
+ this.Main = (args) => QualityQueriesFinder.Main(args);
Review Comment:
Good catch!
##########
src/Lucene.Net.Analysis.Stempel/Egothor.Stemmer/Compile.cs:
##########
@@ -74,6 +81,13 @@ public static class Compile // LUCENENET specific: CA1052
Static holder types sh
private static Trie trie;
/// <summary>
+ /// <para>
+ /// LUCENENET specific: This class is not for direct use. In the Java
implementation
+ /// it's Main method was intended to be called from the command line.
However in .NET a
+ /// method within a DLL can't be directly called from the command line
so we
+ /// provide a <see
href="https://www.nuget.org/packages/lucene-cli";>lucene-cli</see>
+ /// with a command that maps to that method: analysis
stempel-compile-stems.
+ /// </para>
Review Comment:
I know this is the correct way to use an HTML paragraph tag, but I have
found that using a single closed tag between paragraphs also works (`<para/>`)
and replaces the `<p>` tag they use in Java without the need of an additional
closing tag. Since that is the way it is used in most of the other code
comments, could you please change them in this PR, as well?
##########
src/Lucene.Net.Analysis.Kuromoji/Tools/DictionaryBuilder.cs:
##########
@@ -20,6 +20,13 @@ namespace Lucene.Net.Analysis.Ja.Util
* limitations under the License.
*/
+ /// <summary>
+ /// LUCENENET specific: This class is not for direct use. In the Java
implementation
Review Comment:
Technically, the `DictionaryBuilder.Build()` method can be used from a
calling app by calling into the DLL.
And it looks like all of the dependent types are public, so this could be
used as a console app by copy and paste if one were to also bring in all of the
dependencies.
Maybe just delete this doc comment. There is a lot that could go here to
explain how to use this, but I see that the blog post I used to test it is
listed in our docs for this command:
https://lucenenet.apache.org/docs/4.8.0-beta00016/cli/analysis/kuromoji-build-dictionary.html#description.
I also looked up the procedure on ChatGPT, if you are interested:
https://chatgpt.com/share/66f50a60-8e38-8005-9c29-f7633024787c
Maybe we could explain here or on the command below that the instructions on
how to use this are on that page, since if a user starts here, they won't see
the usage instructions.
##########
src/Lucene.Net.Demo/Facet/AssociationsFacetsExample.cs:
##########
@@ -157,7 +157,17 @@ public FacetResult RunDrillDown()
}
- /// <summary>Runs the sum int/float associations examples and prints
the results.</summary>
+ /// <summary>
+ /// Runs the sum int/float associations examples and prints the
results.
+ /// <para>
+ /// LUCENENET specific: This method is not for direct use. In the
Java implementation
Review Comment:
Technically, the demos can be directly used by copying them to a local
console app, so let's change the text. But having a link over to `lucene-cli`
informs anyone looking through the source code that there is another way, so
let's keep that link. For these demos, users might prefer having the actual
code so they can tweak it to see what different changes do than to go over to
`lucene-cli`.
Do note we don't even have a NuGet package for these demos. The only way we
ship them is through `lucene-cli`. Other than that, they are just source code
users can experiment with. But the demo documentation still shows how things
were done in Java and much of it needs to be updated for .NET users instead of
Java users:
https://github.com/apache/lucenenet/blob/master/src/Lucene.Net.Demo/overview.md.
Do note the demos the `lucene-cli` have commands to export the source code
file. That feature could also be improved by accepting a project name from the
user and outputting a `<project name>.csproj` file so it could be directly run
by `dotnet run` once exported. Then we could sensibly update the document with
command line instructions people could run to get the demo source code on the
command line and run it on the command line. You don't have to do that in this
PR, I am just pointing out how this experience could be improved (part of #457
involves investigating improvements like this).
If you wish to revert all of these comments and save the work for #457, I
think that would be fine. Or for now, let's just present `lucene-cli` as an
alternate option to copying the source rather than the only option.
I am not going to repeat this on the others, but this applies to all of the
console apps in the `Lucene.Net.Demo` project.
--
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]
