[
https://issues.apache.org/jira/browse/GEODE-7864?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17101547#comment-17101547
]
ASF GitHub Bot commented on GEODE-7864:
---------------------------------------
jujoramos commented on a change in pull request #5049:
URL: https://github.com/apache/geode/pull/5049#discussion_r421406231
##########
File path:
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/tier/sockets/InterestListEndpointDUnitTest.java
##########
@@ -284,7 +285,7 @@ public static void stopILEndpointServer() {
Iterator iter_prox =
bs.getAcceptor().getCacheClientNotifier().getClientProxies().iterator();
if (iter_prox.hasNext()) {
CacheClientProxy proxy = (CacheClientProxy) iter_prox.next();
- // if (proxy._interestList._keysOfInterest.get("/"+REGION_NAME) !=
null) {
+ // if (proxy._interestList._keysOfInterest.get(SEPARATOR+REGION_NAME)
!= null) {
Review comment:
Maybe just delete this line as it's commented out?.
##########
File path:
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/tier/sockets/InterestListEndpointDUnitTest.java
##########
@@ -398,9 +399,9 @@ public static void
verifyIfNotInterestListEndpointAndThenPut() {
// only one server thats why if and not while
if (iter.hasNext()) {
CacheClientProxy proxy = (CacheClientProxy) iter.next();
- // if (proxy._interestList._keysOfInterest.get("/"+ REGION_NAME) ==
null) {
+ // if (proxy._interestList._keysOfInterest.get(SEPARATOR+ REGION_NAME)
== null) {
Review comment:
Maybe just delete this line as it's commented out?.
##########
File path:
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/tier/sockets/RegionCloseDUnitTest.java
##########
@@ -224,7 +225,7 @@ public String description() {
}
};
GeodeAwaitility.await().untilAsserted(ev);
- // assertNull(c.getRegion("/"+clientMembershipId));
+ // assertNull(c.getRegion(SEPARATOR+clientMembershipId));
Review comment:
Maybe just delete this line as it's commented out?.
##########
File path:
geode-core/src/integrationTest/java/org/apache/geode/internal/jta/functional/CacheJUnitTest.java
##########
@@ -255,7 +257,8 @@ public void testScenario3() throws Exception {
assertEquals("get failed for corresponding put", "\"value1\"", tok);
current_fullpath = jtaObj.currRegion.getFullPath();
- assertEquals("failed retrieving current region fullpath", "/" +
DEFAULT_RGN + "/region1",
+ assertEquals("failed retrieving current region fullpath",
+ SEPARATOR + DEFAULT_RGN + "/region1",
Review comment:
Did you miss this one?, shouldn't it be `SEPARATOR + DEFAULT_RGN +
SEPARATOR + "region1"`?
##########
File path:
geode-core/src/integrationTest/java/org/apache/geode/internal/jta/functional/CacheJUnitTest.java
##########
@@ -245,7 +246,8 @@ public void testScenario3() throws Exception {
jtaObj.getRegionFromCache("region1");
String current_fullpath = jtaObj.currRegion.getFullPath();
- assertEquals("failed retrieving current region fullpath", "/" +
DEFAULT_RGN + "/region1",
+ assertEquals("failed retrieving current region fullpath",
+ SEPARATOR + DEFAULT_RGN + "/region1",
Review comment:
Did you miss this one?, shouldn't it be `SEPARATOR + DEFAULT_RGN +
SEPARATOR + "region1"`?
##########
File path:
geode-core/src/integrationTest/java/org/apache/geode/internal/jta/functional/CacheJUnitTest.java
##########
@@ -276,7 +279,7 @@ public void testScenario3() throws Exception {
current_fullpath = jtaObj.currRegion.getFullPath();
assertEquals("failed retrieving current region fullpath after txn
commit",
- "/" + DEFAULT_RGN + "/region1", current_fullpath);
+ SEPARATOR + DEFAULT_RGN + "/region1", current_fullpath);
Review comment:
Did you miss this one?, shouldn't it be `SEPARATOR + DEFAULT_RGN +
SEPARATOR + "region1"`?
##########
File path:
geode-core/src/distributedTest/java/org/apache/geode/management/QueryDataDUnitTest.java
##########
@@ -313,7 +314,8 @@ public void testErrors() {
distributedSystemMXBean.queryData(regionsNotFoundOnMembersQuery,
member1.getId(), 2);
assertThat(regionsNotFoundOnMembersResult,
isJson(withJsonPath("$.message",
equalTo(
- String.format("Cannot find regions %s in specified members", "/"
+ regionName)))));
+ String.format("Cannot find regions %s in specified members",
+ SEPARATOR + regionName)))));
Review comment:
Scratch that, it's just a test...
##########
File path:
geode-core/src/distributedTest/java/org/apache/geode/management/QueryDataDUnitTest.java
##########
@@ -313,7 +314,8 @@ public void testErrors() {
distributedSystemMXBean.queryData(regionsNotFoundOnMembersQuery,
member1.getId(), 2);
assertThat(regionsNotFoundOnMembersResult,
isJson(withJsonPath("$.message",
equalTo(
- String.format("Cannot find regions %s in specified members", "/"
+ regionName)))));
+ String.format("Cannot find regions %s in specified members",
+ SEPARATOR + regionName)))));
Review comment:
Nitpick: since we're using `String.format` here, wouldn't it be better
(performance wise) to include and extra parameter within the `String` instead
of the concatenation?.
##########
File path:
geode-core/src/integrationTest/java/org/apache/geode/internal/jta/functional/CacheJUnitTest.java
##########
@@ -341,7 +344,7 @@ public void testScenario4() throws Exception {
String current_fullpath = jtaObj.currRegion.getFullPath();
assertEquals(
"failed retrieving current region fullpath after doing
getRegionFromCache(region1)",
- "/" + DEFAULT_RGN + "/region1", current_fullpath);
+ SEPARATOR + DEFAULT_RGN + "/region1", current_fullpath);
Review comment:
Did you miss this one?, shouldn't it be `SEPARATOR + DEFAULT_RGN +
SEPARATOR + "region1"`?
##########
File path:
geode-core/src/integrationTest/java/org/apache/geode/internal/jta/functional/CacheJUnitTest.java
##########
@@ -352,7 +355,8 @@ public void testScenario4() throws Exception {
assertEquals("get value do not match with the put", "\"value1\"", tok);
current_fullpath = jtaObj.currRegion.getFullPath();
- assertEquals("failed retrieving current region fullpath", "/" +
DEFAULT_RGN + "/region1",
+ assertEquals("failed retrieving current region fullpath",
+ SEPARATOR + DEFAULT_RGN + "/region1",
Review comment:
Did you miss this one?, shouldn't it be `SEPARATOR + DEFAULT_RGN +
SEPARATOR + "region1"`?
##########
File path:
geode-core/src/integrationTest/java/org/apache/geode/internal/jta/functional/CacheJUnitTest.java
##########
@@ -454,7 +458,7 @@ public void testScenario5() throws Exception {
current_fullpath = jtaObj.currRegion.getFullPath();
assertEquals("failed retrieving current fullpath, current fullpath: " +
current_fullpath,
- "/" + DEFAULT_RGN + "/region1", current_fullpath);
+ SEPARATOR + DEFAULT_RGN + "/region1", current_fullpath);
Review comment:
Did you miss this one?, shouldn't it be `SEPARATOR + DEFAULT_RGN +
SEPARATOR + "region1"`?
##########
File path:
geode-core/src/integrationTest/java/org/apache/geode/internal/jta/functional/CacheJUnitTest.java
##########
@@ -440,7 +444,7 @@ public void testScenario5() throws Exception {
// now current region should point to region1, as done from within
// getRegionFromCache method...
String current_fullpath = jtaObj.currRegion.getFullPath();
- assertEquals("failed retirieving current fullpath", "/" + DEFAULT_RGN +
"/region1",
+ assertEquals("failed retirieving current fullpath", SEPARATOR +
DEFAULT_RGN + "/region1",
Review comment:
Did you miss this one?, shouldn't it be `SEPARATOR + DEFAULT_RGN +
SEPARATOR + "region1"`?
##########
File path:
geode-core/src/integrationTest/java/org/apache/geode/internal/jta/functional/CacheJUnitTest.java
##########
@@ -372,7 +376,7 @@ public void testScenario4() throws Exception {
current_fullpath = jtaObj.currRegion.getFullPath();
assertEquals("failed retirieving current region fullpath after txn
rollback",
- "/" + DEFAULT_RGN + "/region1", current_fullpath);
+ SEPARATOR + DEFAULT_RGN + "/region1", current_fullpath);
Review comment:
Did you miss this one?, shouldn't it be `SEPARATOR + DEFAULT_RGN +
SEPARATOR + "region1"`?
##########
File path:
geode-core/src/integrationTest/java/org/apache/geode/internal/jta/functional/CacheJUnitTest.java
##########
@@ -577,7 +582,8 @@ public void testScenario7() throws Exception {
assertEquals("get value mismatch with put", "\"value1\"", tok);
current_fullpath = jtaObj.currRegion.getFullPath();
- assertEquals("failed retrieving current region fullpath", "/" +
DEFAULT_RGN + "/region1",
+ assertEquals("failed retrieving current region fullpath",
+ SEPARATOR + DEFAULT_RGN + "/region1",
Review comment:
Did you miss this one?, shouldn't it be `SEPARATOR + DEFAULT_RGN +
SEPARATOR + "region1"`?
##########
File path:
geode-core/src/integrationTest/java/org/apache/geode/internal/jta/functional/CacheJUnitTest.java
##########
@@ -590,7 +596,7 @@ public void testScenario7() throws Exception {
current_fullpath = jtaObj.currRegion.getFullPath();
assertEquals("failed retrieving current region fullpath after txn
commit, fullpath is: "
- + current_region, "/" + DEFAULT_RGN + "/region1", current_fullpath);
+ + current_region, SEPARATOR + DEFAULT_RGN + "/region1",
current_fullpath);
Review comment:
Did you miss this one?, shouldn't it be `SEPARATOR + DEFAULT_RGN +
SEPARATOR + "region1"`?
##########
File path:
geode-core/src/integrationTest/java/org/apache/geode/internal/jta/functional/CacheJUnitTest.java
##########
@@ -505,7 +509,7 @@ public void testScenario5() throws Exception {
String current_fullpath = jtaObj.currRegion.getFullPath();
assertEquals(
"failed retrieving current fullpath after rollback, fullpath is: "
+ current_fullpath,
- "/" + DEFAULT_RGN + "/region1", current_fullpath);
+ SEPARATOR + DEFAULT_RGN + "/region1", current_fullpath);
Review comment:
Did you miss this one?, shouldn't it be `SEPARATOR + DEFAULT_RGN +
SEPARATOR + "region1"`?
----------------------------------------------------------------
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
> Code improvement refactoring
> ----------------------------
>
> Key: GEODE-7864
> URL: https://issues.apache.org/jira/browse/GEODE-7864
> Project: Geode
> Issue Type: Improvement
> Reporter: Nabarun Nag
> Priority: Major
> Time Spent: 13h 10m
> Remaining Estimate: 0h
>
> This is a placeholder ticket.
> * this is used to do refactoring.
> * this ticket number is used to number the commit message.
> * this ticket will never be closed.
> * it will be used to mark improvements like correcting spelling mistakes,
> efficient java code, etc.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
