You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucenenet.apache.org by GitBox <gi...@apache.org> on 2021/02/12 17:52:53 UTC

[GitHub] [lucenenet] rclabo opened a new pull request #420: Reviewed classification tests and made minor changes. Part of issue #259

rclabo opened a new pull request #420:
URL: https://github.com/apache/lucenenet/pull/420


   Reviewed classification tests. Made minor changes and uncommented 1 test and verified it passes.  See issue #259


----------------------------------------------------------------
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.

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



[GitHub] [lucenenet] rclabo commented on a change in pull request #420: Reviewed classification tests and made minor changes. Part of issue #259

Posted by GitBox <gi...@apache.org>.
rclabo commented on a change in pull request #420:
URL: https://github.com/apache/lucenenet/pull/420#discussion_r575486624



##########
File path: src/Lucene.Net.Tests.Classification/Utils/DataSplitterTest.cs
##########
@@ -32,20 +32,20 @@ namespace Lucene.Net.Classification
      */
     public class DataSplitterTest : Util.LuceneTestCase
     {
-        private AtomicReader _originalIndex;
-        private RandomIndexWriter _indexWriter;
+        private AtomicReader originalIndex;
+        private RandomIndexWriter indexWriter;
         private Directory _dir;

Review comment:
       Of course.  That was a brain fart.  I must have gotten interrupted. Sorry about that.




----------------------------------------------------------------
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.

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



[GitHub] [lucenenet] NightOwl888 merged pull request #420: Reviewed classification tests and made minor changes. Part of issue #259

Posted by GitBox <gi...@apache.org>.
NightOwl888 merged pull request #420:
URL: https://github.com/apache/lucenenet/pull/420


   


----------------------------------------------------------------
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.

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



[GitHub] [lucenenet] NightOwl888 commented on a change in pull request #420: Reviewed classification tests and made minor changes. Part of issue #259

Posted by GitBox <gi...@apache.org>.
NightOwl888 commented on a change in pull request #420:
URL: https://github.com/apache/lucenenet/pull/420#discussion_r575467606



##########
File path: src/Lucene.Net.Tests.Classification/Utils/DataSplitterTest.cs
##########
@@ -32,20 +32,20 @@ namespace Lucene.Net.Classification
      */
     public class DataSplitterTest : Util.LuceneTestCase
     {
-        private AtomicReader _originalIndex;
-        private RandomIndexWriter _indexWriter;
+        private AtomicReader originalIndex;
+        private RandomIndexWriter indexWriter;
         private Directory _dir;

Review comment:
       While either fields with or without an underscore are fine (without underscore preferred), we should use the same convention within the context of a single file. Please change this to `dir` as well.
   
   Do note that some classes in `Lucene.Net.Classification` also use a leading underscore. No objection if you wish to change those as well.




----------------------------------------------------------------
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.

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



[GitHub] [lucenenet] rclabo commented on a change in pull request #420: Reviewed classification tests and made minor changes. Part of issue #259

Posted by GitBox <gi...@apache.org>.
rclabo commented on a change in pull request #420:
URL: https://github.com/apache/lucenenet/pull/420#discussion_r575490504



##########
File path: src/Lucene.Net.Tests.Classification/Utils/DataSplitterTest.cs
##########
@@ -32,20 +32,20 @@ namespace Lucene.Net.Classification
      */
     public class DataSplitterTest : Util.LuceneTestCase
     {
-        private AtomicReader _originalIndex;
-        private RandomIndexWriter _indexWriter;
+        private AtomicReader originalIndex;
+        private RandomIndexWriter indexWriter;
         private Directory _dir;

Review comment:
       Sounds good. I will clean up Lucene.Net.Classification leading underscores in a separate pull request.




----------------------------------------------------------------
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.

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