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