[
https://issues.apache.org/jira/browse/GEODE-3584?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16373638#comment-16373638
]
Patrick Rhomberg commented on GEODE-3584:
-----------------------------------------
A few thoughts, in no particular order:
First, I love the refactoring effort! So thanks for getting that ball rolling.
This looks like it's going to touch a lot of non-internal, non-private methods
and classes. To adhere to SemVer, we need to not break any public-facing API
until the next major release. I definitely think these classes could use some
reorganization, but we should start a conversation on d...@geode.apache.org
(and maybe user@, too) if we want to, say, deprecate use of the inner-class
builders in factor of them becoming their own first-class classes, as it were.
It seems like, with how densely connected some of these things are, we should
probably split this up into several tickets. I poked through Ken's feature
branch and took a run at what I had hoped were low-hanging refactor fruits, and
I immediately realized how non-trivial this effort is going to be. Potential
tickets that come to my mind are: several tickets for promoting inner classes
to their own classes, a ticket for reconciling the differences in the Command
enums, and another ticket or two to reduce code duplication.
I'll go ahead and open and start the Command reconciliation ticket as a child
to this ticket.
> Refactor ServerLauncher and LocatorLauncher to eliminate code duplication
> -------------------------------------------------------------------------
>
> Key: GEODE-3584
> URL: https://issues.apache.org/jira/browse/GEODE-3584
> Project: Geode
> Issue Type: Improvement
> Components: gfsh
> Affects Versions: 1.2.0
> Reporter: Kenneth Howe
> Priority: Major
> Attachments: GEODE-3584-1, GEODE-3584-WIP, GEODE-3584-WIP-2,
> GEODE-3584-WIP-3, GEODE-3584-WIP-4
>
>
> There is some duplication of code in the Launcher classes that can be
> eliminated. Both classes have methods such as getBindAddress (called
> getServerBindAddress in ServerLauncher) that could be hoisted into
> AbstractLauncher class that they both extend. The same goes for the inner
> State classes of the Launchers; they have methods that could be moved to
> AbstractLauncher.ServiceState.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
