[GitHub] [solr] epugh commented on a diff in pull request #1206: SOLR-16564: Migrate to using DEFAULT_TEST_COLLECTION_NAME constant through out tests

["epugh (via GitHub)" <gi...@apache.org>]

epugh commented on code in PR #1206:
URL: https://github.com/apache/solr/pull/1206#discussion_r1103813300


##########
solr/core/src/test/org/apache/solr/cloud/LeaderVoteWaitTimeoutTest.java:
##########
@@ -179,74 +179,78 @@ public void basicTest() throws Exception {
 
   @Test
   public void testMostInSyncReplicasCanWinElection() throws Exception {
-    final String collectionName = "collection1";
-    CollectionAdminRequest.createCollection(collectionName, 1, 3)
+    // final String collectionName = "collection1";

Review Comment:
   I think the value of this PR is to use the same pattern everywhere (even though maybe the diff is bigger).    I want to only have a variable called collectionName if that is somehow important to the function of the test..  when I see the variable I immediately think, oh, it might change, and I need to think about that as I read the test.    Where as seeing the constant everywhere means I know that the name of the collection isn't important to the function of the test...   So thats why I made these changes, and yeah, it did make the diff bigger...   Does that argument sway your opinion?   I could split up this patch into multiple patches too if that was better...   My end goal on this PR is to use the same constant the same way everywhere that it makes sense......



-- 
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: issues-unsubscribe@solr.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org