NightOwl888 commented on code in PR #1051:
URL: https://github.com/apache/lucenenet/pull/1051#discussion_r1860754285


##########
src/Lucene.Net.TestFramework/Util/LuceneTestCase.cs:
##########
@@ -67,7 +67,7 @@ namespace Lucene.Net.Util
     ///
     /// <para>
     /// The preferred way to specify class (suite-level) setup/cleanup is to 
use
-    /// static methods annotated with <see cref="OneTimeSetUp"/> and <see 
cref="OneTimeTearDown"/>. Any
+    /// static methods annotated with <see 
cref="NUnit.Framework.OneTimeSetUpAttribute"/> and <see 
cref="NUnit.Framework.OneTimeTearDownAttribute"/>. Any

Review Comment:
   This documentation is incorrect for Lucene.NET because NUnit calls static 
methods annotated with the attributes in the wrong order. It should read:
   
   ```
   The preferred way to specify class (suite-level) setup/cleanup is to override
   <see cref="OneTimeSetUp"/> and <see cref="OneTimeTearDown"/>. Be sure
   to call <c>base.OneTimeSetUp()</c> BEFORE you initialize your class and
   call <c>base.OneTimeTearDown()</c> AFTER you clean up your class. NUnit
   will find the <see cref="NUnit.Framework.OneTimeSetUpAttribute"/> and
   <see cref="NUnit.Framework.OneTimeTearDownAttribute"/> of the base class,
   so using them on the <see cref="OneTimeSetUp"/> and
   <see cref="OneTimeTearDown"/> method overrides is not strictly required. Any
   ```



##########
src/Lucene.Net.Tests/Search/TestFieldCache.cs:
##########
@@ -107,9 +107,9 @@ public override void TearDown()
 
         // LUCENENET: Changed to non-static because NewIndexWriterConfig is 
non-static

Review Comment:
   Please remove this comment. It was added during the attempt to use xUnit and 
no longer reflects the reason why this method is not static (due to NUnit 
calling it in the wrong order). We already explain that in `LuceneTestCase`, so 
I don't see a need to repeat it on every overload.



##########
src/Lucene.Net.Tests/Search/Spans/TestFieldMaskingSpanQuery.cs:
##########
@@ -62,9 +62,9 @@ protected internal static Field GetField(string name, string 
value)
         /// Is non-static because NewIndexWriterConfig is no longer static.

Review Comment:
   Please remove this comment. It was added during the attempt to use xUnit and 
no longer reflects the reason why this method is not static (due to NUnit 
calling it in the wrong order). We already explain that in `LuceneTestCase`, so 
I don't see a need to repeat it on every overload.



##########
src/Lucene.Net.Tests.Facet/Taxonomy/TestTaxonomyFacetAssociations.cs:
##########
@@ -53,9 +53,9 @@ public class TestTaxonomyFacetAssociations : FacetTestCase
         /// Is non-static because Similarity and TimeZone are not static.

Review Comment:
   Please remove this comment. It was added during the attempt to use xUnit and 
no longer reflects the reason why this method is not static (due to NUnit 
calling it in the wrong order). We already explain that in `LuceneTestCase`, so 
I don't see a need to repeat it on every overload.



##########
src/Lucene.Net.Tests/Search/TestMultiTermQueryRewrites.cs:
##########
@@ -52,9 +52,9 @@ public class TestMultiTermQueryRewrites : LuceneTestCase
         /// Is non-static because Similarity and TimeZone are not static.

Review Comment:
   Please remove this comment. It was added during the attempt to use xUnit and 
no longer reflects the reason why this method is not static (due to NUnit 
calling it in the wrong order). We already explain that in `LuceneTestCase`, so 
I don't see a need to repeat it on every overload.



##########
src/Lucene.Net.Tests/Search/Spans/TestBasics.cs:
##########
@@ -102,9 +102,9 @@ public override void Reset()
         /// Is non-static because NewIndexWriterConfig is no longer static.

Review Comment:
   Please remove this comment. It was added during the attempt to use xUnit and 
no longer reflects the reason why this method is not static (due to NUnit 
calling it in the wrong order). We already explain that in `LuceneTestCase`, so 
I don't see a need to repeat it on every overload.



##########
src/Lucene.Net.Tests/Search/TestNGramPhraseQuery.cs:
##########
@@ -38,9 +38,9 @@ public class TestNGramPhraseQuery : LuceneTestCase
         /// Is non-static because Similarity and TimeZone are not static.

Review Comment:
   Please remove this comment. It was added during the attempt to use xUnit and 
no longer reflects the reason why this method is not static (due to NUnit 
calling it in the wrong order). We already explain that in `LuceneTestCase`, so 
I don't see a need to repeat it on every overload.



##########
src/Lucene.Net.Tests/Search/TestMinShouldMatch2.cs:
##########
@@ -67,9 +67,9 @@ public class TestMinShouldMatch2 : LuceneTestCase
         /// Is non-static because Similarity and TimeZone are not static.

Review Comment:
   Please remove this comment. It was added during the attempt to use xUnit and 
no longer reflects the reason why this method is not static (due to NUnit 
calling it in the wrong order). We already explain that in `LuceneTestCase`, so 
I don't see a need to repeat it on every overload.



##########
src/Lucene.Net.Tests/Search/TestBooleanMinShouldMatch.cs:
##########
@@ -50,9 +50,9 @@ public class TestBooleanMinShouldMatch : LuceneTestCase
         /// Is non-static because NewStringField is no longer static.

Review Comment:
   Please remove this comment. It was added during the attempt to use xUnit and 
no longer reflects the reason why this method is not static (due to NUnit 
calling it in the wrong order). We already explain that in `LuceneTestCase`, so 
I don't see a need to repeat it on every overload.



##########
src/Lucene.Net.Tests/Search/TestNumericRangeQuery64.cs:
##########
@@ -67,9 +67,9 @@ public class TestNumericRangeQuery64 : LuceneTestCase
         /// Is non-static because NewIndexWriterConfig is no longer static.

Review Comment:
   Please remove this comment. It was added during the attempt to use xUnit and 
no longer reflects the reason why this method is not static (due to NUnit 
calling it in the wrong order). We already explain that in `LuceneTestCase`, so 
I don't see a need to repeat it on every overload.



##########
src/Lucene.Net.Tests/Search/TestTermVectors.cs:
##########
@@ -53,9 +53,9 @@ public class TestTermVectors : LuceneTestCase
         /// Is non-static because NewIndexWriterConfig is no longer static.

Review Comment:
   Please remove this comment. It was added during the attempt to use xUnit and 
no longer reflects the reason why this method is not static (due to NUnit 
calling it in the wrong order). We already explain that in `LuceneTestCase`, so 
I don't see a need to repeat it on every overload.



##########
src/Lucene.Net.Tests/Search/TestNumericRangeQuery32.cs:
##########
@@ -67,9 +67,9 @@ public class TestNumericRangeQuery32 : LuceneTestCase
         /// Is non-static because NewIndexWriterConfig is no longer static.

Review Comment:
   Please remove this comment. It was added during the attempt to use xUnit and 
no longer reflects the reason why this method is not static (due to NUnit 
calling it in the wrong order). We already explain that in `LuceneTestCase`, so 
I don't see a need to repeat it on every overload.



##########
src/Lucene.Net.Tests/Search/TestPrefixInBooleanQuery.cs:
##########
@@ -51,9 +51,9 @@ public class TestPrefixInBooleanQuery : LuceneTestCase
         /// Is non-static because Similarity and TimeZone are not static.

Review Comment:
   Please remove this comment. It was added during the attempt to use xUnit and 
no longer reflects the reason why this method is not static (due to NUnit 
calling it in the wrong order). We already explain that in `LuceneTestCase`, so 
I don't see a need to repeat it on every overload.



-- 
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: dev-unsubscr...@lucenenet.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to