[
https://issues.apache.org/jira/browse/GEODE-3271?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16096976#comment-16096976
]
ASF GitHub Bot commented on GEODE-3271:
---------------------------------------
Github user PurelyApplied commented on the issue:
https://github.com/apache/geode/pull/647
+1 as it stands.
Unimportant nitpicks, rambling observations, and "it could be even better
if...":
* `punePort` appears in these tests a lot. I have no idea what `pune` is
supposed to mean. Maybe `dsIdPort` would be better, glancing at the
implementation of `createFirstLocatorWithDSId`?
* There are five instances in `WANCommandsTestBase` that has a loop like
this
```
for (GatewaySender s : senders) {
if (s.getId().equals(senderId)) {
sender = (AbstractGatewaySender) s;
break;
}
}
```
that my hatred of `break` as function control thinks it would read a lot
better as
```
AbstractGatewaySender sender =
(AbstractGatewaySender) senders.stream().filter(s ->
s.getId().equalsIgnoreCase(senderId)).findFirst().orElse(null);
```
* I'd take the Inspection-suggested change in `WANCommandBaseTest.java:489`
to turn the string `indexOf(...) != -1` to be `contains(...)`.
> Refactor WanCommands
> --------------------
>
> Key: GEODE-3271
> URL: https://issues.apache.org/jira/browse/GEODE-3271
> Project: Geode
> Issue Type: Sub-task
> Components: gfsh
> Reporter: Emily Yeh
> Assignee: Emily Yeh
>
> {{WanCommands.java}} is a large class that contains multiple commands. Each
> command should be refactored into a separate class, and the methods shared by
> the commands should be refactored into a new and appropriately named class of
> their own.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
