It sounds like we're on the same page. The current behavior requires two pieces from a user who wants to add commands: 1) implement CommandMarker 2) Add a system property pointing to the package of your class, or provide a META-INF.services file pointing to your class
The proposed behavior is to drop 2) and rely on 1) alone. On May 25, 2017 12:55 PM, "Udo Kohlmeyer" <ukohlme...@pivotal.io> wrote: What I meant was, given our hammer of choice for GFSH is Spring Shell, all command definitions should be of type CommandMarker. Not sure how it loads the correct CommandMarker classes (for different commands). But my gut feel is, have single way of doing something... KISS On 5/25/17 12:22, Jared Stewart wrote: > Can you clarify - I'm proposing we use *implements CommandMarker* alone as > the way to specify commands. What do we gain be *also* requiring a > META-INF.services file? > > On May 25, 2017 12:18 PM, "Udo Kohlmeyer" <ukohlme...@pivotal.io> wrote: > > imo, I think that GFSH is Spring Shell, it should really only be commands > that are registered inside of META-INF.services .. aka implements > CommandMarker. > > This way we has standard simple way to specify commands. > > --Udo > > > > On 5/25/17 11:52, Jared Stewart wrote: > > I would like to propose that we eliminate the “user-command-packages” >> system property, in favor of scanning the entire classpath to find >> commands. >> >> To give more detail, Geode currently supports three ways to load GFSH >> commands: >> Scan the classpath for commands in "org.apache.geode.management.i >> nternal.cli.commands” >> Scan the classpath for commands in a package specified by a user via the >> “user-command-packages” system property. >> Scan the classpath for commands registered in files inside >> META-INF.services (e.g. "geode-core/src/test/resources >> /META-INF/services/org.springframework.shell.core.CommandMarker”) >> >> We have some bespoke code responsible for scanning the classpath >> (ClasspathScanLoadHelper) that I am in the process of replacing with >> FastClasspathScanner (GEODE-2989). Since FastClasspathScanner will no >> longer need to eagerly load all of the classes in the target package, I >> don’t see any reason why a user should need to specify a >> “user-command-package” property or a “META-INF/services” file. Instead, >> we >> should just scan the whole classpath and register any commands that we >> find. >> >> Thoughts? >> >>
