[jira] [Commented] (GEODE-3230) Delete unused commands in CliStrings

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3230?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16123921#comment-16123921
 ] 

ASF subversion and git services commented on GEODE-3230:


Commit a7f29525df2981c1c99abac96ea83cb965295970 in geode's branch 
refs/heads/develop from YehEmily
[ https://git-wip-us.apache.org/repos/asf?p=geode.git;h=a7f2952 ]

GEODE-3230: Cleaning up unused (Cli)Strings

This closes #651


> Delete unused commands in CliStrings
> 
>
> Key: GEODE-3230
> URL: https://issues.apache.org/jira/browse/GEODE-3230
> Project: Geode
>  Issue Type: Improvement
>  Components: gfsh
>Reporter: Emily Yeh
>Assignee: Emily Yeh
>
> There are a lot of commands in {{CliStrings}} that aren't used. For example, 
> {{start manager}} has a whole set of associated commands - 
> {{START_MANAGER__MEMBERNAME}}, {{START_MANAGER__DIR}}, 
> {{START_MANAGER__CLASSPATH}}, etc.) that don't seem to be used anywhere. 
> These commands should be deleted to clean up the code.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (GEODE-3230) Delete unused commands in CliStrings

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3230?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16111651#comment-16111651
 ] 

ASF GitHub Bot commented on GEODE-3230:
---

Github user pdxrunner commented on a diff in the pull request:

https://github.com/apache/geode/pull/651#discussion_r130982335
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/DataCommandFunction.java
 ---
@@ -633,8 +631,8 @@ public DataCommandResult put(String key, String value, 
boolean putIfAbsent, Stri
   try {
 keyObject = getClassObject(key, keyClass);
   } catch (ClassNotFoundException e) {
-return DataCommandResult.createPutResult(key, null, null,
-"ClassNotFoundException " + keyClass, false);
+return DataCommandResult.createPutResult(key, null, 
e.getException(),
--- End diff --

I don't understand the context of this change - the message was 
"ClassNotFound" but now it's "key not found" with a key class? 


> Delete unused commands in CliStrings
> 
>
> Key: GEODE-3230
> URL: https://issues.apache.org/jira/browse/GEODE-3230
> Project: Geode
>  Issue Type: Improvement
>  Components: gfsh
>Reporter: Emily Yeh
>Assignee: Emily Yeh
>
> There are a lot of commands in {{CliStrings}} that aren't used. For example, 
> {{start manager}} has a whole set of associated commands - 
> {{START_MANAGER__MEMBERNAME}}, {{START_MANAGER__DIR}}, 
> {{START_MANAGER__CLASSPATH}}, etc.) that don't seem to be used anywhere. 
> These commands should be deleted to clean up the code.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (GEODE-3230) Delete unused commands in CliStrings

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3230?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16111490#comment-16111490
 ] 

ASF GitHub Bot commented on GEODE-3230:
---

Github user YehEmily commented on a diff in the pull request:

https://github.com/apache/geode/pull/651#discussion_r130959623
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/DataCommandFunction.java
 ---
@@ -633,8 +631,8 @@ public DataCommandResult put(String key, String value, 
boolean putIfAbsent, Stri
   try {
 keyObject = getClassObject(key, keyClass);
   } catch (ClassNotFoundException e) {
-return DataCommandResult.createPutResult(key, null, null,
-"ClassNotFoundException " + keyClass, false);
+return DataCommandResult.createPutResult(key, null, 
e.getException(),
--- End diff --

Fixed - thanks for your feedback!


> Delete unused commands in CliStrings
> 
>
> Key: GEODE-3230
> URL: https://issues.apache.org/jira/browse/GEODE-3230
> Project: Geode
>  Issue Type: Improvement
>  Components: gfsh
>Reporter: Emily Yeh
>Assignee: Emily Yeh
>
> There are a lot of commands in {{CliStrings}} that aren't used. For example, 
> {{start manager}} has a whole set of associated commands - 
> {{START_MANAGER__MEMBERNAME}}, {{START_MANAGER__DIR}}, 
> {{START_MANAGER__CLASSPATH}}, etc.) that don't seem to be used anywhere. 
> These commands should be deleted to clean up the code.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (GEODE-3230) Delete unused commands in CliStrings

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3230?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16109401#comment-16109401
 ] 

ASF GitHub Bot commented on GEODE-3230:
---

Github user jaredjstewart commented on a diff in the pull request:

https://github.com/apache/geode/pull/651#discussion_r130677475
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/management/internal/cli/Launcher.java 
---
@@ -99,7 +99,6 @@ protected Launcher() {
 this.allowedCommandLineCommands.add(CliStrings.START_JCONSOLE);
 this.allowedCommandLineCommands.add(CliStrings.START_JVISUALVM);
 this.allowedCommandLineCommands.add(CliStrings.START_LOCATOR);
-this.allowedCommandLineCommands.add(CliStrings.START_MANAGER);
--- End diff --

Can you explain the reason this change?


> Delete unused commands in CliStrings
> 
>
> Key: GEODE-3230
> URL: https://issues.apache.org/jira/browse/GEODE-3230
> Project: Geode
>  Issue Type: Improvement
>  Components: gfsh
>Reporter: Emily Yeh
>Assignee: Emily Yeh
>
> There are a lot of commands in {{CliStrings}} that aren't used. For example, 
> {{start manager}} has a whole set of associated commands - 
> {{START_MANAGER__MEMBERNAME}}, {{START_MANAGER__DIR}}, 
> {{START_MANAGER__CLASSPATH}}, etc.) that don't seem to be used anywhere. 
> These commands should be deleted to clean up the code.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (GEODE-3230) Delete unused commands in CliStrings

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3230?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16109400#comment-16109400
 ] 

ASF GitHub Bot commented on GEODE-3230:
---

Github user jaredjstewart commented on a diff in the pull request:

https://github.com/apache/geode/pull/651#discussion_r130678413
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/DataCommandFunction.java
 ---
@@ -633,8 +631,8 @@ public DataCommandResult put(String key, String value, 
boolean putIfAbsent, Stri
   try {
 keyObject = getClassObject(key, keyClass);
   } catch (ClassNotFoundException e) {
-return DataCommandResult.createPutResult(key, null, null,
-"ClassNotFoundException " + keyClass, false);
+return DataCommandResult.createPutResult(key, null, 
e.getException(),
--- End diff --

I think we can pass the Exception `e` itself rather than `e.getException()`.


> Delete unused commands in CliStrings
> 
>
> Key: GEODE-3230
> URL: https://issues.apache.org/jira/browse/GEODE-3230
> Project: Geode
>  Issue Type: Improvement
>  Components: gfsh
>Reporter: Emily Yeh
>Assignee: Emily Yeh
>
> There are a lot of commands in {{CliStrings}} that aren't used. For example, 
> {{start manager}} has a whole set of associated commands - 
> {{START_MANAGER__MEMBERNAME}}, {{START_MANAGER__DIR}}, 
> {{START_MANAGER__CLASSPATH}}, etc.) that don't seem to be used anywhere. 
> These commands should be deleted to clean up the code.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (GEODE-3230) Delete unused commands in CliStrings

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3230?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16109402#comment-16109402
 ] 

ASF GitHub Bot commented on GEODE-3230:
---

Github user jaredjstewart commented on a diff in the pull request:

https://github.com/apache/geode/pull/651#discussion_r130677047
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/process/MBeanProcessController.java
 ---
@@ -260,6 +265,7 @@ private JMXServiceURL getJMXServiceURL() throws 
AttachNotSupportedException, IOE
 // need to load the management-agent and get the address
 
 final String javaHome = 
vm.getSystemProperties().getProperty("java.home");
+assertState(StringUtils.isNotBlank(javaHome), 
CliStrings.JAVA_HOME_NOT_FOUND_ERROR_MESSAGE);
--- End diff --

I worry about changing the exception the exception thrown here (before I 
think it would have thrown an `IOException`).


> Delete unused commands in CliStrings
> 
>
> Key: GEODE-3230
> URL: https://issues.apache.org/jira/browse/GEODE-3230
> Project: Geode
>  Issue Type: Improvement
>  Components: gfsh
>Reporter: Emily Yeh
>Assignee: Emily Yeh
>
> There are a lot of commands in {{CliStrings}} that aren't used. For example, 
> {{start manager}} has a whole set of associated commands - 
> {{START_MANAGER__MEMBERNAME}}, {{START_MANAGER__DIR}}, 
> {{START_MANAGER__CLASSPATH}}, etc.) that don't seem to be used anywhere. 
> These commands should be deleted to clean up the code.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (GEODE-3230) Delete unused commands in CliStrings

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3230?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16102047#comment-16102047
 ] 

ASF GitHub Bot commented on GEODE-3230:
---

Github user YehEmily commented on a diff in the pull request:

https://github.com/apache/geode/pull/651#discussion_r129654481
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java
 ---
@@ -639,29 +639,34 @@ public Result compactOfflineDiskStore(
   String errorString = CliStrings.format(
   
CliStrings.COMPACT_OFFLINE_DISK_STORE__MSG__ERROR_WHILE_COMPACTING_DISKSTORE_0_WITH_1_REASON_2,
   diskStoreName, fieldsMessage);
-  result = ResultBuilder.createUserErrorResult(errorString);
+  result = ResultBuilder.createUserErrorResult(CliStrings.format(
--- End diff --

Good catch! I fixed this and updated the PR. Thank you!


> Delete unused commands in CliStrings
> 
>
> Key: GEODE-3230
> URL: https://issues.apache.org/jira/browse/GEODE-3230
> Project: Geode
>  Issue Type: Improvement
>  Components: gfsh
>Reporter: Emily Yeh
>Assignee: Emily Yeh
>
> There are a lot of commands in {{CliStrings}} that aren't used. For example, 
> {{start manager}} has a whole set of associated commands - 
> {{START_MANAGER__MEMBERNAME}}, {{START_MANAGER__DIR}}, 
> {{START_MANAGER__CLASSPATH}}, etc.) that don't seem to be used anywhere. 
> These commands should be deleted to clean up the code.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (GEODE-3230) Delete unused commands in CliStrings

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3230?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16102014#comment-16102014
 ] 

ASF GitHub Bot commented on GEODE-3230:
---

Github user jaredjstewart commented on a diff in the pull request:

https://github.com/apache/geode/pull/651#discussion_r129647633
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java
 ---
@@ -639,29 +639,34 @@ public Result compactOfflineDiskStore(
   String errorString = CliStrings.format(
   
CliStrings.COMPACT_OFFLINE_DISK_STORE__MSG__ERROR_WHILE_COMPACTING_DISKSTORE_0_WITH_1_REASON_2,
   diskStoreName, fieldsMessage);
-  result = ResultBuilder.createUserErrorResult(errorString);
+  result = ResultBuilder.createUserErrorResult(CliStrings.format(
--- End diff --

This looks to me like it's going to build a strange error message that I 
expect will look something like this:: 

```
An error occurred while doing compaction: "While compacting disk store={0} 
{1}. Reason: {2}"
``` 

I think {0} and {1} will be filled in but not {2}.


> Delete unused commands in CliStrings
> 
>
> Key: GEODE-3230
> URL: https://issues.apache.org/jira/browse/GEODE-3230
> Project: Geode
>  Issue Type: Improvement
>  Components: gfsh
>Reporter: Emily Yeh
>Assignee: Emily Yeh
>
> There are a lot of commands in {{CliStrings}} that aren't used. For example, 
> {{start manager}} has a whole set of associated commands - 
> {{START_MANAGER__MEMBERNAME}}, {{START_MANAGER__DIR}}, 
> {{START_MANAGER__CLASSPATH}}, etc.) that don't seem to be used anywhere. 
> These commands should be deleted to clean up the code.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (GEODE-3230) Delete unused commands in CliStrings

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/GEODE-3230?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16098974#comment-16098974
 ] 

ASF GitHub Bot commented on GEODE-3230:
---

GitHub user YehEmily opened a pull request:

https://github.com/apache/geode/pull/651

GEODE-3230: Cleaning up unused (Cli)Strings

[View the JIRA ticket 
here.](https://issues.apache.org/jira/browse/GEODE-3230)

`CliStrings.java` contained a lot of Strings that were not used anywhere in 
the code. In this PR, each of these seemingly unused Strings were examined to 
find potential uses of them in the code. Strings that did seem useful were 
inserted into the code as needed. Strings that didn't seem to be useful were 
deleted. Strings that don't seem to be used, but are used in the closed side, 
were preserved and tagged as such.

(Note: An attempt was made to use all Strings that weren't currently being 
used in parts of the code that seemed appropriate, but most of these were 
removed because they broke tests.)

**Testing Status**: **Precheckin running**

### For all changes:
- [x] JIRA ticket

- [x] PR rebased

- [x] Initial commit is squashed and single

- [x] `gradlew build` runs cleanly

- [x] Tests updated to verify changes

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/YehEmily/geode GEODE-3230

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/geode/pull/651.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #651


commit 06dcd340a33c83cab7b4700a3c56e8fe780d0964
Author: YehEmily 
Date:   2017-07-21T21:29:55Z

GEODE-3230: Cleaning up unused (Cli)Strings




> Delete unused commands in CliStrings
> 
>
> Key: GEODE-3230
> URL: https://issues.apache.org/jira/browse/GEODE-3230
> Project: Geode
>  Issue Type: Improvement
>  Components: gfsh
>Reporter: Emily Yeh
>Assignee: Emily Yeh
>
> There are a lot of commands in {{CliStrings}} that aren't used. For example, 
> {{start manager}} has a whole set of associated commands - 
> {{START_MANAGER__MEMBERNAME}}, {{START_MANAGER__DIR}}, 
> {{START_MANAGER__CLASSPATH}}, etc.) that don't seem to be used anywhere. 
> These commands should be deleted to clean up the code.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)