jinmeiliao commented on a change in pull request #6423:
URL: https://github.com/apache/geode/pull/6423#discussion_r625849420
##########
File path:
geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/CommandManager.java
##########
@@ -176,16 +177,15 @@ private void loadPluginCommands() {
private void loadCommands() {
Set<String> userCommandPackages = getUserCommandPackages();
Set<String> packagesToScan = new HashSet<>(userCommandPackages);
- packagesToScan.add("org.apache.geode");
packagesToScan.add("org.springframework.shell.converters");
- packagesToScan.add(GfshCommand.class.getPackage().getName());
- packagesToScan.add(VersionCommand.class.getPackage().getName());
+ // packagesToScan.add(GfshCommand.class.getPackage().getName());
+ // packagesToScan.add(VersionCommand.class.getPackage().getName());
// Create one scanner to be used everywhere
try (ClasspathScanLoadHelper scanner = new
ClasspathScanLoadHelper(packagesToScan)) {
loadUserCommands(scanner, userCommandPackages);
Review comment:
we can just pass in the `userCommandsPackages` for this method, and
create the scanner inside the method, we don't need all the previous lines.
##########
File path:
geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/CommandManager.java
##########
@@ -176,16 +177,15 @@ private void loadPluginCommands() {
private void loadCommands() {
Set<String> userCommandPackages = getUserCommandPackages();
Set<String> packagesToScan = new HashSet<>(userCommandPackages);
- packagesToScan.add("org.apache.geode");
packagesToScan.add("org.springframework.shell.converters");
- packagesToScan.add(GfshCommand.class.getPackage().getName());
- packagesToScan.add(VersionCommand.class.getPackage().getName());
+ // packagesToScan.add(GfshCommand.class.getPackage().getName());
+ // packagesToScan.add(VersionCommand.class.getPackage().getName());
// Create one scanner to be used everywhere
try (ClasspathScanLoadHelper scanner = new
ClasspathScanLoadHelper(packagesToScan)) {
loadUserCommands(scanner, userCommandPackages);
loadPluginCommands();
- loadGeodeCommands(scanner);
+ // loadGeodeCommands(scanner);
loadConverters(scanner);
Review comment:
since we are using service loader to load all the geode converters, we
don't need to pass in the scanner here anymore. Inside the `loadConverters` we
can create a scanner only for `"org.springframework.shell.converters"` package
and add those converters.
##########
File path:
geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/CommandManager.java
##########
@@ -176,16 +177,15 @@ private void loadPluginCommands() {
private void loadCommands() {
Set<String> userCommandPackages = getUserCommandPackages();
Set<String> packagesToScan = new HashSet<>(userCommandPackages);
- packagesToScan.add("org.apache.geode");
packagesToScan.add("org.springframework.shell.converters");
- packagesToScan.add(GfshCommand.class.getPackage().getName());
- packagesToScan.add(VersionCommand.class.getPackage().getName());
+ // packagesToScan.add(GfshCommand.class.getPackage().getName());
+ // packagesToScan.add(VersionCommand.class.getPackage().getName());
// Create one scanner to be used everywhere
try (ClasspathScanLoadHelper scanner = new
ClasspathScanLoadHelper(packagesToScan)) {
loadUserCommands(scanner, userCommandPackages);
loadPluginCommands();
Review comment:
this code actually loads all geode commands, right? let's rename it.
--
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
