You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "PakhomovAlexander (via GitHub)" <gi...@apache.org> on 2024/05/14 09:45:34 UTC
[PR] IGNITE-22108 Rename options in CLI [ignite-3]
PakhomovAlexander opened a new pull request, #3750:
URL: https://github.com/apache/ignite-3/pull/3750
(no comment)
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22108 Rename options in CLI [ignite-3]
Posted by "PakhomovAlexander (via GitHub)" <gi...@apache.org>.
PakhomovAlexander merged PR #3750:
URL: https://github.com/apache/ignite-3/pull/3750
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22108 Rename options in CLI [ignite-3]
Posted by "valepakh (via GitHub)" <gi...@apache.org>.
valepakh commented on code in PR #3750:
URL: https://github.com/apache/ignite-3/pull/3750#discussion_r1601721400
##########
modules/cli/src/main/java/org/apache/ignite/internal/cli/commands/Options.java:
##########
@@ -155,48 +154,34 @@ public static final class Constants {
/** Profile copy from option long name. */
public static final String PROFILE_COPY_FROM_OPTION = "--copy-from";
- /** Profile copy from option short name. */
- public static final String PROFILE_COPY_FROM_OPTION_SHORT = "-c";
-
/** Profile copy from option description. */
public static final String PROFILE_COPY_FROM_OPTION_DESC = "Profile whose content will be copied to new one";
/** Profile activate option long name. */
public static final String PROFILE_ACTIVATE_OPTION = "--activate";
- /** Profile activate option short name. */
- public static final String PROFILE_ACTIVATE_OPTION_SHORT = "-a";
-
/** Profile activate option description. */
- public static final String PROFILE_ACTIVATE_OPTION_DESC = "Activate new profile as current or not";
+ public static final String PROFILE_ACTIVATE_OPTION_DESC = "Activate new profile as current or not. "
+ + "By activating a profile, you set profile settings for the current session";
/** Cluster management node name option long name. */
public static final String CMG_NODE_NAME_OPTION = "--cmg-node";
- /** Cluster management node name option short name. */
- public static final String CMG_NODE_NAME_OPTION_SHORT = "-c";
-
/** Cluster management node name option description. */
public static final String CMG_NODE_NAME_OPTION_DESC = "Name of the node (repeat like '--cmg-node node1 --cmg-node node2' "
+ "to specify more than one node) that will host the Cluster Management Group."
- + "If omitted, then --meta-storage-node values will also supply the nodes for the Cluster Management Group.";
+ + "If omitted, then --ms-node values will also supply the nodes for the Cluster Management Group.";
/** Meta storage management node name option long name. */
- public static final String META_STORAGE_NODE_NAME_OPTION = "--meta-storage-node";
-
- /** Meta storage node name option short name. */
- public static final String META_STORAGE_NODE_NAME_OPTION_SHORT = "-m";
+ public static final String META_STORAGE_NODE_NAME_OPTION = "--ms-node";
Review Comment:
> How do you think if we keep both `--ms-node` and `--meta-storage-node`?
I think it could be confusing for users
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22108 Rename options in CLI [ignite-3]
Posted by "PakhomovAlexander (via GitHub)" <gi...@apache.org>.
PakhomovAlexander commented on code in PR #3750:
URL: https://github.com/apache/ignite-3/pull/3750#discussion_r1601715650
##########
modules/cli/src/main/java/org/apache/ignite/internal/cli/commands/Options.java:
##########
@@ -155,48 +154,34 @@ public static final class Constants {
/** Profile copy from option long name. */
public static final String PROFILE_COPY_FROM_OPTION = "--copy-from";
- /** Profile copy from option short name. */
- public static final String PROFILE_COPY_FROM_OPTION_SHORT = "-c";
-
/** Profile copy from option description. */
public static final String PROFILE_COPY_FROM_OPTION_DESC = "Profile whose content will be copied to new one";
/** Profile activate option long name. */
public static final String PROFILE_ACTIVATE_OPTION = "--activate";
- /** Profile activate option short name. */
- public static final String PROFILE_ACTIVATE_OPTION_SHORT = "-a";
-
/** Profile activate option description. */
- public static final String PROFILE_ACTIVATE_OPTION_DESC = "Activate new profile as current or not";
+ public static final String PROFILE_ACTIVATE_OPTION_DESC = "Activate new profile as current or not. "
+ + "By activating a profile, you set profile settings for the current session";
/** Cluster management node name option long name. */
public static final String CMG_NODE_NAME_OPTION = "--cmg-node";
- /** Cluster management node name option short name. */
- public static final String CMG_NODE_NAME_OPTION_SHORT = "-c";
-
/** Cluster management node name option description. */
public static final String CMG_NODE_NAME_OPTION_DESC = "Name of the node (repeat like '--cmg-node node1 --cmg-node node2' "
+ "to specify more than one node) that will host the Cluster Management Group."
- + "If omitted, then --meta-storage-node values will also supply the nodes for the Cluster Management Group.";
+ + "If omitted, then --ms-node values will also supply the nodes for the Cluster Management Group.";
/** Meta storage management node name option long name. */
- public static final String META_STORAGE_NODE_NAME_OPTION = "--meta-storage-node";
-
- /** Meta storage node name option short name. */
- public static final String META_STORAGE_NODE_NAME_OPTION_SHORT = "-m";
+ public static final String META_STORAGE_NODE_NAME_OPTION = "--ms-node";
Review Comment:
How do you think if we keep both `--ms-node` and `--meta-storage-node`?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22108 Rename options in CLI [ignite-3]
Posted by "valepakh (via GitHub)" <gi...@apache.org>.
valepakh commented on code in PR #3750:
URL: https://github.com/apache/ignite-3/pull/3750#discussion_r1601638171
##########
docs/_docs/installation/installing-using-docker.adoc:
##########
@@ -76,13 +76,13 @@ x-ignite-def:
services:
node1:
<< : *ignite-def
- command: --node-name node1
+ command: --nod-name node1
Review Comment:
```suggestion
command: --node node1
```
##########
modules/cli/src/main/java/org/apache/ignite/internal/cli/commands/Options.java:
##########
@@ -155,48 +154,34 @@ public static final class Constants {
/** Profile copy from option long name. */
public static final String PROFILE_COPY_FROM_OPTION = "--copy-from";
- /** Profile copy from option short name. */
- public static final String PROFILE_COPY_FROM_OPTION_SHORT = "-c";
-
/** Profile copy from option description. */
public static final String PROFILE_COPY_FROM_OPTION_DESC = "Profile whose content will be copied to new one";
/** Profile activate option long name. */
public static final String PROFILE_ACTIVATE_OPTION = "--activate";
- /** Profile activate option short name. */
- public static final String PROFILE_ACTIVATE_OPTION_SHORT = "-a";
-
/** Profile activate option description. */
- public static final String PROFILE_ACTIVATE_OPTION_DESC = "Activate new profile as current or not";
+ public static final String PROFILE_ACTIVATE_OPTION_DESC = "Activate new profile as current or not. "
+ + "By activating a profile, you set profile settings for the current session";
/** Cluster management node name option long name. */
public static final String CMG_NODE_NAME_OPTION = "--cmg-node";
- /** Cluster management node name option short name. */
- public static final String CMG_NODE_NAME_OPTION_SHORT = "-c";
-
/** Cluster management node name option description. */
public static final String CMG_NODE_NAME_OPTION_DESC = "Name of the node (repeat like '--cmg-node node1 --cmg-node node2' "
+ "to specify more than one node) that will host the Cluster Management Group."
- + "If omitted, then --meta-storage-node values will also supply the nodes for the Cluster Management Group.";
+ + "If omitted, then --ms-node values will also supply the nodes for the Cluster Management Group.";
/** Meta storage management node name option long name. */
- public static final String META_STORAGE_NODE_NAME_OPTION = "--meta-storage-node";
-
- /** Meta storage node name option short name. */
- public static final String META_STORAGE_NODE_NAME_OPTION_SHORT = "-m";
+ public static final String META_STORAGE_NODE_NAME_OPTION = "--ms-node";
Review Comment:
I don't think this abbreviation is something user is expecting, maybe we could keep the full option name?
##########
modules/cli/src/main/java/org/apache/ignite/internal/cli/commands/cluster/unit/ClusterUnitUndeployCommand.java:
##########
@@ -47,7 +46,7 @@ public class ClusterUnitUndeployCommand extends BaseCommand implements Callable<
private String id;
/** Unit version. */
- @Option(names = {VERSION_OPTION, UNIT_VERSION_OPTION_SHORT},
+ @Option(names = VERSION_OPTION,
description = UNIT_VERSION_OPTION_DESC,
required = true)
Review Comment:
```suggestion
@Option(names = VERSION_OPTION, description = UNIT_VERSION_OPTION_DESC, required = true)
```
##########
docs/_docs/installation/installing-using-docker.adoc:
##########
@@ -76,13 +76,13 @@ x-ignite-def:
services:
node1:
<< : *ignite-def
- command: --node-name node1
+ command: --nod-name node1
ports:
- 10300:10300
- 3344:3344
node2:
<< : *ignite-def
- command: --node-name node2
+ command: --nod-name node2
Review Comment:
```suggestion
command: --node node2
```
##########
modules/cli/src/main/java/org/apache/ignite/internal/cli/core/repl/completer/cluster/ClusterUrlDynamicCompleterFactory.java:
##########
@@ -25,7 +25,7 @@
import org.apache.ignite.internal.cli.core.repl.completer.StringDynamicCompleter;
import org.apache.ignite.internal.cli.core.repl.registry.NodeNameRegistry;
-/** Factory for --cluster-endpoint-url option completer. */
+/** Factory for --url option completer. */
Review Comment:
```suggestion
/** Factory for --url option completer for cluster commands. */
```
##########
modules/cli/src/main/java/org/apache/ignite/internal/cli/commands/sql/SqlReplCommand.java:
##########
@@ -77,7 +75,7 @@
*/
@Command(name = "sql", description = "Executes SQL query")
public class SqlReplCommand extends BaseCommand implements Runnable {
- @Option(names = {JDBC_URL_OPTION, JDBC_URL_OPTION_SHORT}, required = true,
+ @Option(names = JDBC_URL_OPTION, required = true,
descriptionKey = JDBC_URL_KEY, description = JDBC_URL_OPTION_DESC)
Review Comment:
```suggestion
@Option(names = JDBC_URL_OPTION, required = true, descriptionKey = JDBC_URL_KEY, description = JDBC_URL_OPTION_DESC)
```
##########
modules/cli/src/test/java/org/apache/ignite/internal/cli/commands/UrlOptionsNegativeTest.java:
##########
@@ -91,8 +91,8 @@
public class UrlOptionsNegativeTest {
private static final String NODE_URL = "http://localhost:10300";
- private static final String CLUSTER_URL_OPTION = "--cluster-endpoint-url=";
- private static final String NODE_URL_OPTION = "--node-url=";
+ private static final String CLUSTER_URL_OPTION = "--url=";
+ private static final String NODE_URL_OPTION = "--url=";
Review Comment:
Combine this into single `URL_OPTION`?
##########
modules/cli/src/main/java/org/apache/ignite/internal/cli/commands/sql/SqlReplCommand.java:
##########
@@ -91,8 +89,7 @@ private static class ExecOptions {
@Parameters(index = "0", description = "SQL query to execute", defaultValue = Option.NULL_VALUE)
private String command;
- @Option(names = {SCRIPT_FILE_OPTION, SCRIPT_FILE_OPTION_SHORT}, description = SCRIPT_FILE_OPTION_SHORT,
- defaultValue = Option.NULL_VALUE)
+ @Option(names = SCRIPT_FILE_OPTION, defaultValue = Option.NULL_VALUE)
Review Comment:
I think `description` is missing here
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org