sk0x50 commented on a change in pull request #31:
URL: https://github.com/apache/ignite-3/pull/31#discussion_r561657765
##########
File path:
modules/cli/src/main/java/org/apache/ignite/cli/CliPathsConfigLoader.java
##########
@@ -27,51 +27,96 @@
import javax.inject.Singleton;
import org.apache.ignite.cli.builtins.SystemPathResolver;
+/**
+ * Due to the nature of Ignite CLI tool it can be runned in the environment
without ignitecfg file at all.
+ * This class created to simplify the managing of the different cases:
+ * <ul>
+ * <li>When user download binary and run it to manage any existence remote
cluster</li>
+ * <li>When user download binary and run 'init' to deploy Ignite
distribution on the current machine</li>
+ * <li>When user install it by system package</li>
+ * </ul>
+ */
@Singleton
public class CliPathsConfigLoader {
+ /** System paths' resolver. */
+ private final SystemPathResolver pathRslvr;
- private final SystemPathResolver pathResolver;
- private final String version;
+ /** Ignite CLI tool version. */
+ private final String ver;
+ /**
+ * Creates new loader.
+ *
+ * @param pathRslvr System paths' resolver.
+ * @param cliVerInfo CLI version info provider.
+ */
@Inject
- public CliPathsConfigLoader(SystemPathResolver pathResolver,
- CliVersionInfo cliVersionInfo) {
- this.pathResolver = pathResolver;
- this.version = cliVersionInfo.version;
+ public CliPathsConfigLoader(
+ SystemPathResolver pathRslvr,
+ CliVersionInfo cliVerInfo
+ ) {
+ this.pathRslvr = pathRslvr;
+ this.ver = cliVerInfo.ver;
}
+ /**
+ * Loads Ignite paths config from file if exists.
+ *
+ * @return IgnitePaths if config file exists, empty otherwise.
+ */
public Optional<IgnitePaths> loadIgnitePathsConfig() {
if (configFilePath().toFile().exists())
- return
Optional.of(CliPathsConfigLoader.readConfigFile(configFilePath(), version));
+ return Optional.of(readConfigFile(configFilePath(), ver));
return Optional.empty();
}
+ /**
+ * Loads Ignite paths configuration if config file exists or failed
otherwise.
+ *
+ * @return IgnitePaths or throw exception, if no config file exists.
+ */
Review comment:
Perhaps, it would be good to explicitly declare the type of Exception:
`@throws IgniteCLIException If the configuration file does not exist.`
##########
File path: modules/cli/src/main/java/org/apache/ignite/cli/IgnitePaths.java
##########
@@ -20,43 +20,96 @@
import java.io.File;
import java.nio.file.Path;
+/**
+ * The main resolver of Ignite paths for the current installation (like bin,
work and etc. dirs).
+ * Current Ignite distributive has the following dirs structure:
+ * <ul>
+ * <li>bin</li>
+ * <ul>
+ * <li>${version}</li>
+ * <ul>
+ * <li>cli</li>
+ * <li>libs</li>
+ * </ul>
+ * </ul>
+ * <li>work</li>
+ * <ul>
+ * <li>config</li>
+ * <ul>
+ * <li>default-config.xml</li>
+ * </ul>
+ * <li>cli</li>
+ * <ul>
+ * <li>pids</li>
+ * </ul>
+ * <li>modules.json</li>
+ * </ul>
+ * </ul>
+ */
public class IgnitePaths {
-
+ /**
+ * Path to Ignite bin directory.
+ * Bin directory contains jar artifacts for Ignite server and CLI modules.
+ */
public final Path binDir;
+
+ /**
+ * Work directory for Ignite server and CLI operation.
+ */
public final Path workDir;
- private final String version;
- public IgnitePaths(Path binDir, Path workDir, String version) {
+ /**
+ * Ignite CLI version.
+ * Also, the same version will be used for addressing any binaries inside
bin dir
+ */
+ private final String ver;
+
+ /**
+ * Creates resolved ignite paths instance from Ignite CLI version and base
dirs paths.
+ *
+ * @param binDir Bin directory.
+ * @param workDir Work directory.
+ * @param ver Ignite CLI version.
+ */
+ public IgnitePaths(Path binDir, Path workDir, String ver) {
this.binDir = binDir;
this.workDir = workDir;
- this.version = version;
+ this.ver = ver;
}
-
+ /** Path where CLI module artifacts will be placed. */
Review comment:
One-line Javadoc comments allowed and preferred for fields. I'm not sure
that this can be applied to methods.
##########
File path: modules/cli/src/main/java/org/apache/ignite/cli/IgnitePaths.java
##########
@@ -20,43 +20,96 @@
import java.io.File;
import java.nio.file.Path;
+/**
+ * The main resolver of Ignite paths for the current installation (like bin,
work and etc. dirs).
+ * Current Ignite distributive has the following dirs structure:
+ * <ul>
+ * <li>bin</li>
+ * <ul>
+ * <li>${version}</li>
+ * <ul>
+ * <li>cli</li>
+ * <li>libs</li>
+ * </ul>
+ * </ul>
+ * <li>work</li>
+ * <ul>
+ * <li>config</li>
+ * <ul>
+ * <li>default-config.xml</li>
+ * </ul>
+ * <li>cli</li>
+ * <ul>
+ * <li>pids</li>
+ * </ul>
+ * <li>modules.json</li>
+ * </ul>
+ * </ul>
+ */
public class IgnitePaths {
-
+ /**
+ * Path to Ignite bin directory.
+ * Bin directory contains jar artifacts for Ignite server and CLI modules.
+ */
public final Path binDir;
+
+ /**
+ * Work directory for Ignite server and CLI operation.
Review comment:
This javadoc can be a one-liner.
##########
File path:
modules/cli/src/main/java/org/apache/ignite/cli/CliPathsConfigLoader.java
##########
@@ -27,51 +27,96 @@
import javax.inject.Singleton;
import org.apache.ignite.cli.builtins.SystemPathResolver;
+/**
+ * Due to the nature of Ignite CLI tool it can be runned in the environment
without ignitecfg file at all.
+ * This class created to simplify the managing of the different cases:
+ * <ul>
+ * <li>When user download binary and run it to manage any existence remote
cluster</li>
+ * <li>When user download binary and run 'init' to deploy Ignite
distribution on the current machine</li>
+ * <li>When user install it by system package</li>
+ * </ul>
+ */
@Singleton
public class CliPathsConfigLoader {
+ /** System paths' resolver. */
+ private final SystemPathResolver pathRslvr;
- private final SystemPathResolver pathResolver;
- private final String version;
+ /** Ignite CLI tool version. */
+ private final String ver;
+ /**
+ * Creates new loader.
+ *
+ * @param pathRslvr System paths' resolver.
+ * @param cliVerInfo CLI version info provider.
+ */
@Inject
- public CliPathsConfigLoader(SystemPathResolver pathResolver,
- CliVersionInfo cliVersionInfo) {
- this.pathResolver = pathResolver;
- this.version = cliVersionInfo.version;
+ public CliPathsConfigLoader(
+ SystemPathResolver pathRslvr,
+ CliVersionInfo cliVerInfo
+ ) {
+ this.pathRslvr = pathRslvr;
+ this.ver = cliVerInfo.ver;
}
+ /**
+ * Loads Ignite paths config from file if exists.
+ *
+ * @return IgnitePaths if config file exists, empty otherwise.
+ */
public Optional<IgnitePaths> loadIgnitePathsConfig() {
if (configFilePath().toFile().exists())
- return
Optional.of(CliPathsConfigLoader.readConfigFile(configFilePath(), version));
+ return Optional.of(readConfigFile(configFilePath(), ver));
return Optional.empty();
}
+ /**
+ * Loads Ignite paths configuration if config file exists or failed
otherwise.
+ *
+ * @return IgnitePaths or throw exception, if no config file exists.
+ */
public IgnitePaths loadIgnitePathsOrThrowError() {
Optional<IgnitePaths> ignitePaths = loadIgnitePathsConfig();
+
if (ignitePaths.isPresent()) {
if (!ignitePaths.get().validateDirs())
Review comment:
Even though there is only one instruction under this condition, it
requires brackets because it occupies two lines.
##########
File path: modules/cli/src/main/java/org/apache/ignite/cli/CommandFactory.java
##########
@@ -21,16 +21,25 @@
import io.micronaut.context.ApplicationContext;
import picocli.CommandLine;
+/**
+ * Picocli command factory for initialize commands and DI dependencies.
+ */
public class CommandFactory implements CommandLine.IFactory {
+ /** DI application context. */
+ private final ApplicationContext applicationCtx;
- private final ApplicationContext applicationContext;
-
- public CommandFactory(ApplicationContext applicationContext) {
- this.applicationContext = applicationContext;
+ /**
+ * Creates new command factory.
+ *
+ * @param applicationCtx DI application context.
+ */
+ public CommandFactory(ApplicationContext applicationCtx) {
+ this.applicationCtx = applicationCtx;
}
+ /** {inheritDoc} */
Review comment:
Please change it to `{@inheritDoc}` here and below.
----------------------------------------------------------------
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:
[email protected]
