cpoerschke commented on a change in pull request #2152:
URL: https://github.com/apache/lucene-solr/pull/2152#discussion_r548124061



##########
File path: solr/core/src/test/org/apache/solr/cloud/HttpPartitionTest.java
##########
@@ -548,9 +548,6 @@ protected int sendDoc(int docId, Integer minRf, SolrClient 
solrClient, String co
     doc.addField("a_t", "hello" + docId);
 
     UpdateRequest up = new UpdateRequest();
-    if (minRf != null) {
-      up.setParam(UpdateRequest.MIN_REPFACT, String.valueOf(minRf));
-    }

Review comment:
       Thanks for the code pointers and detailed explanation!
   
   How about we scope this pull request to remove `minRf` in almost all code 
paths i.e. `sendDoc` gets its unused argument removed in both the replication 
factor and http partition tests but `sendDocsWithRetry` remains unchanged for 
now since as you say it's used in other test classes too?
   
   It appears that the `minRf` argument in `sendDocsWithRetry` is indeed unused 
already but it might be clearer to tidy that up in a separate follow-up pull 
request rather than mix it in here at this time.

##########
File path: solr/core/src/test/org/apache/solr/cloud/ReplicationFactorTest.java
##########
@@ -461,38 +447,18 @@ protected void doDelete(UpdateRequest req, String msg, 
int expectedRf, int retri
 
   protected int sendDoc(int docId, int minRf) throws Exception {
     UpdateRequest up = new UpdateRequest();
-    boolean minRfExplicit = maybeAddMinRfExplicitly(minRf, up);
     SolrInputDocument doc = new SolrInputDocument();
     doc.addField(id, String.valueOf(docId));
     doc.addField("a_t", "hello" + docId);
     up.add(doc);
-    return runAndGetAchievedRf(up, minRfExplicit, minRf);
+    return runAndGetAchievedRf(up);

Review comment:
       Alright, so with `maybeAddMinRfExplicitly` gone and the `minRf` argument 
in `runAndGetAchievedRf` gone the `minRf` argument of `sendDoc` becomes unused, 
right? I wonder how the code would work out if `sendDoc` had its `minRf` 
argument taken away too? Bit like peeling an onion this here i.e. multiple 
layers (but it hopefully won't make anyone cry).




----------------------------------------------------------------
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:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Reply via email to