You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/06/13 11:41:44 UTC
[GitHub] [solr] cpoerschke commented on a diff in pull request #900: SOLR-16244: Update typos and grammar
cpoerschke commented on code in PR #900:
URL: https://github.com/apache/solr/pull/900#discussion_r895591283
##########
solr/core/src/test/org/apache/solr/cloud/DistribDocExpirationUpdateProcessorTest.java:
##########
@@ -209,20 +209,20 @@ private void runTest() throws Exception {
req.process(cluster.getSolrClient(), COLLECTION).getResults().getNumFound());
}
- // now that we've confrmed the basics work, let's check some fine grain stuff...
+ // now that we've confirmed the basics work, let's check some fine grain stuff...
- // first off, sanity check that this special docId doesn't some how already exist
+ // first off, sanity check that this special docId doesn't somehow already exist
Review Comment:
```suggestion
// first off, check that this special docId doesn't somehow already exist
```
##########
solr/core/src/test/org/apache/solr/cloud/FullSolrCloudDistribCmdsTest.java:
##########
@@ -575,15 +575,15 @@ public long testIndexQueryDeleteHierarchical() throws Exception {
assertEquals(
topDocsNum, cloudClient.query(new SolrQuery("type_s:parent")).getResults().getNumFound());
- // childs
+ // children
assertEquals(
topDocsNum * childsNum,
cloudClient.query(new SolrQuery("type_s:child")).getResults().getNumFound());
- // grandchilds
+ // grandchildren
//
- // each topDoc has t childs where each child has x = 0 + 2 + 4 + ..(t-1)*2 grands
- // x = 2 * (1 + 2 + 3 +.. (t-1)) => arithmetic summ of t-1
+ // each topDoc has t children where each child has x = 0 + 2 + 4 + ..(t-1)*2 grands
Review Comment:
```suggestion
// each topDoc has t children where each child has x = 0 + 2 + 4 + ..(t-1)*2 grandchildren
```
##########
solr/core/src/test/org/apache/solr/cloud/DeleteNodeTest.java:
##########
@@ -70,21 +70,21 @@ public void test() throws Exception {
create.setCreateNodeSet(StrUtils.join(l, ','));
cloudClient.request(create);
state = cloudClient.getClusterState();
- String node2bdecommissioned = l.get(0);
+ String node2BeDecommissioned = l.get(0);
Review Comment:
```suggestion
String nodeToBeDecommissioned = l.get(0);
```
##########
solr/core/src/test/org/apache/solr/cloud/ReplaceNodeTest.java:
##########
@@ -84,7 +84,7 @@ public void test() throws Exception {
ArrayList<String> l = new ArrayList<>(liveNodes);
Collections.shuffle(l, random());
String emptyNode = l.remove(0);
- String node2bdecommissioned = l.get(0);
+ String node2BeDecommissioned = l.get(0);
Review Comment:
```suggestion
String nodeToBeDecommissioned = l.get(0);
```
##########
solr/core/src/test/org/apache/solr/cloud/OverseerStatusTest.java:
##########
@@ -64,7 +64,7 @@ public void test() throws Exception {
// When cluster state updates are distributed, Overseer doesn't see them and doesn't report
// stats on them.
if (!cluster.getOpenOverseer().getDistributedClusterStateUpdater().isDistributedStateUpdate()) {
- // Note the "create" key here is in a different map from the "create" key above. Above it's
+ // Note the "create" key here is in a different map from the "create" key above. Above its
Review Comment:
```suggestion
// Note the "create" key here is in a different map from the "create" key above. Above it's from the
```
##########
solr/core/src/test/org/apache/solr/cloud/TestMiniSolrCloudClusterSSL.java:
##########
@@ -113,7 +113,7 @@ public void testNoSsl() throws Exception {
public void testNoSslButSillyClientAuth() throws Exception {
// this combination doesn't really make sense, since ssl==false the clientauth option will be
- // ignored but we test it anyway for completeness of sanity checking the behavior of code that
+ // ignored, but we test it anyway for completeness of sanity checking the behavior of code that
Review Comment:
```suggestion
// ignored, but we test it anyway for completeness of checking the behavior of code that
```
##########
solr/core/src/test/org/apache/solr/cloud/TestCloudPivotFacet.java:
##########
@@ -405,8 +405,8 @@ private void assertPivotStats(String message, PivotField constraint, QueryRespon
}
/**
- * Verify that the PivotFields we're lookin at doesn't violate any of the expected behaviors based
- * on the <code>TRACE_*</code> params found in the base params
+ * Verify that the PivotFields we're looking at doesn't violate any of the expected behaviors
Review Comment:
```suggestion
* Verify that the PivotFields we're looking at don't violate any of the expected behaviors
```
##########
solr/core/src/test/org/apache/solr/cloud/TestMiniSolrCloudClusterSSL.java:
##########
@@ -398,8 +398,8 @@ private static int doHeadRequest(final CloseableHttpClient client, final String
private static CloseableHttpClient getSslAwareClientWithNoClientCerts() throws Exception {
// NOTE: This method explicitly does *NOT* use HttpClientUtil code because that
- // will muck with the global static HttpClientBuilder / SchemeRegistryProvider
- // and we can't do that and still test the entire purpose of what we are trying to test here.
+ // will muck with the global static HttpClientBuilder / SchemeRegistryProvider, and
Review Comment:
```suggestion
// will change the global static HttpClientBuilder / SchemeRegistryProvider, and
```
##########
solr/core/src/test/org/apache/solr/cloud/ReplicationFactorTest.java:
##########
@@ -136,7 +136,8 @@ protected void testRf2NotUsingDirectUpdates() throws Exception {
sendNonDirectUpdateRequestReplicaWithRetry(leader, up, 2, testCollectionName);
sendNonDirectUpdateRequestReplicaWithRetry(replicas.get(0), up, 2, testCollectionName);
- // Insure nothing is tricky about a delete where only one shard needs to delete anything.
+ // Insure nothing is tricky about a delete operation where only one shard needs to delete
Review Comment:
```suggestion
// Ensure nothing is tricky about a delete operation where only one shard needs to delete
```
##########
solr/core/src/test/org/apache/solr/cloud/TestRandomFlRTGCloud.java:
##########
@@ -77,7 +77,7 @@ public class TestRandomFlRTGCloud extends SolrCloudTestCase {
private static final List<HttpSolrClient> CLIENTS =
Collections.synchronizedList(new ArrayList<>(5));
- /** Always included in fl so we can vet what doc we're looking at */
+ /** Always included in fl, so we can vet what doc we're looking at */
Review Comment:
```suggestion
/** Always included in fl, so we can check what doc we're looking at */
```
##########
solr/core/src/test/org/apache/solr/cloud/TestTolerantUpdateProcessorCloud.java:
##########
@@ -162,7 +162,7 @@ public static void createMiniSolrCloudCluster() throws Exception {
fail("unexpected shard: " + shardName);
}
}
- assertEquals("Should be exactly one server left (nost hosting either shard)", 1, urlMap.size());
Review Comment:
another note-to-self bookmark for later w.r.t. `not` or not
##########
solr/core/src/test/org/apache/solr/cloud/OverseerTest.java:
##########
@@ -447,7 +447,7 @@ public void tearDown() throws Exception {
* obtained.
*/
private void createCollection(String collection, int numShards) throws Exception {
- // Create collection znode before having ClusterStateUpdater create state.json below it or it
+ // Create collection znode before having ClusterStateUpdater create state.json below it, or it
Review Comment:
```suggestion
// Create collection znode before having ClusterStateUpdater create state.json below, or it
```
##########
solr/core/src/test/org/apache/solr/cloud/TestRandomFlRTGCloud.java:
##########
@@ -215,7 +215,7 @@ public void testCoverage() throws Exception {
// and a specific Jira for fixing this is listed as a comment
final List<String> knownBugs =
Arrays.asList(
- "child" // way to complicatd to vet with this test, see SOLR-9379 instead
+ "child" // way to complicated to vet with this test, see SOLR-9379 instead
Review Comment:
```suggestion
"child" // way too complicated to check with this test, see SOLR-9379 instead
```
##########
solr/core/src/test/org/apache/solr/cloud/TestPullReplicaWithAuth.java:
##########
@@ -133,7 +133,7 @@ public void testPKIAuthWorksForPullReplication() throws Exception {
QueryResponse statsResponse =
queryWithBasicAuth(
pullReplicaClient, new SolrQuery("qt", "/admin/plugins", "stats", "true"));
- // adds is a gauge, which is null for PULL replicas
+ // adds metric is a gauge, which is null for PULL replicas
Review Comment:
```suggestion
// the 'adds' metric is a gauge, which is null for PULL replicas
```
##########
solr/core/src/test/org/apache/solr/cloud/TestStressInPlaceUpdates.java:
##########
@@ -423,7 +423,8 @@ public void run() {
QueryResponse response = clients.get(clientId).query(params);
if (response.getResults().size() == 0) {
- // there's no info we can get back with a delete, so not much we can check
+ // there's no info we can get back with from a delete operation, so not much we
Review Comment:
```suggestion
// there's no info we can get back from a delete operation, so not much we
```
##########
solr/core/src/test/org/apache/solr/cloud/TestSkipOverseerOperations.java:
##########
@@ -222,28 +222,28 @@ public boolean matches(SortedSet<String> oldLiveNodes, SortedSet<String> newLive
// See comment in testSkipLeaderOperations() above why this assert is skipped
if (!cluster.getOpenOverseer().getDistributedClusterStateUpdater().isDistributedStateUpdate()) {
// 2 for recovering state, 4 for active state
- assertEquals(getNumStateOpeations(resp) + 6, getNumStateOpeations(resp2));
+ assertEquals(getNumStateOperations(resp) + 6, getNumStateOperations(resp2));
}
CollectionAdminRequest.deleteCollection(collection).process(cluster.getSolrClient());
}
/**
* Returns the value corresponding to stat: "overseer_operations", "leader", "requests" This stat
- * (see {@link org.apache.solr.cloud.api.collections.OverseerStatusCmd} is updated when the
+ * (see {@link org.apache.solr.cloud.api.collections.OverseerStatusCmd} ) is updated when the
Review Comment:
```suggestion
* (see {@link org.apache.solr.cloud.api.collections.OverseerStatusCmd}) is updated when the
```
##########
solr/core/src/test/org/apache/solr/cloud/TestStressInPlaceUpdates.java:
##########
@@ -454,31 +455,33 @@ public void run() {
}
// Some things we can assert about any Doc returned from solr,
- // even if it's newer then our (expected) model information...
+ // even if it's newer than our (expected) model information...
assertTrue(
msg + " ...how did a doc in solr get a non positive intVal?", 0 < intVal);
assertTrue(
msg + " ...how did a doc in solr get a non positive longVal?", 0 < longVal);
assertEquals(
msg
- + " ...intVal and longVal in solr doc are internally (modulo) inconsistent w/eachother",
+ + " ...intVal and longVal in solr doc are internally (modulo) inconsistent w/each other",
Review Comment:
```suggestion
+ " ...intVal and longVal in solr doc are internally (modulo) inconsistent with each other",
```
##########
solr/core/src/test/org/apache/solr/cloud/TestStressInPlaceUpdates.java:
##########
@@ -583,7 +586,7 @@ public void run() {
assertEquals(msg, expected.version, actual.getFieldValue("_version_"));
assertTrue(msg + " doc exists in solr, but version is negative???", 0 < expected.version);
- // also sanity check the model (which we already know matches the doc)
+ // sanity check the model (which we already know matches the doc)
Review Comment:
```suggestion
// check the model (which we already know matches the doc)
```
##########
solr/core/src/test/org/apache/solr/cloud/SegmentTerminateEarlyTestState.java:
##########
@@ -35,7 +35,7 @@ class SegmentTerminateEarlyTestState {
static final String KEY_FIELD = "id";
- // for historic reasons, this is refered to as a "timestamp" field, but in actuallity is just an
+ // for historic reasons, this is referred to as a "timestamp" field, but in actuality is just an
Review Comment:
```suggestion
// for historic reasons, this is referred to as a "timestamp" field, but actually it is just an
```
##########
solr/core/src/test/org/apache/solr/cloud/TestCloudDeleteByQuery.java:
##########
@@ -173,7 +173,7 @@ private static void createMiniSolrCloudCluster() throws Exception {
fail("unexpected shard: " + shardName);
}
}
- assertEquals("Should be exactly one server left (nost hosting either shard)", 1, urlMap.size());
+ assertEquals("Should be exactly one server left (hosting either shard)", 1, urlMap.size());
Review Comment:
note-to-self bookmark for later: could `nost` have been a typo of `not` here?
--
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