Uploaded image for project: 'Tajo'
  1. Tajo
  2. TAJO-1979

Usage of some TajoShellCommand is omitted

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Trivial
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.12.0, 0.11.1
    • Component/s: SQL Shell
    • Labels:

      Description

      I found that Usage of some TajoShellCommand is omitted.

        Activity

        Hide
        githubbot ASF GitHub Bot added a comment -

        GitHub user dkhwangbo opened a pull request:

        https://github.com/apache/tajo/pull/862

        TAJO-1979: Usage of some TajoShellCommand is omitted

        Write omitted usage of TajoShellCommand.

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

        $ git pull https://github.com/dkhwangbo/tajo TAJO-1979

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

        https://github.com/apache/tajo/pull/862.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 #862


        commit 395ba2ea49e1c5ea3f971a2c5389adf711967274
        Author: Dongkyu Hwangbo <hwangbodk@gmail.com>
        Date: 2015-11-16T05:48:05Z

        initial commit


        Show
        githubbot ASF GitHub Bot added a comment - GitHub user dkhwangbo opened a pull request: https://github.com/apache/tajo/pull/862 TAJO-1979 : Usage of some TajoShellCommand is omitted Write omitted usage of TajoShellCommand. You can merge this pull request into a Git repository by running: $ git pull https://github.com/dkhwangbo/tajo TAJO-1979 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tajo/pull/862.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 #862 commit 395ba2ea49e1c5ea3f971a2c5389adf711967274 Author: Dongkyu Hwangbo <hwangbodk@gmail.com> Date: 2015-11-16T05:48:05Z initial commit
        Hide
        githubbot ASF GitHub Bot added a comment -

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

        https://github.com/apache/tajo/pull/862#discussion_r44901940

        — Diff: tajo-cli/src/main/java/org/apache/tajo/cli/tsql/commands/ConnectDatabaseCommand.java —
        @@ -63,7 +63,7 @@ public void invoke(String[] cmd) throws Exception {

        @Override
        public String getUsage() {

        • return "";
          + return "[database_name]";
            • End diff –

        How about use ```DBNAME``` instead of ```database_name```? TSQL already uses ```DBNAME``` to print help descriptions with ```\help``` parameter.

        Show
        githubbot ASF GitHub Bot added a comment - Github user blrunner commented on a diff in the pull request: https://github.com/apache/tajo/pull/862#discussion_r44901940 — Diff: tajo-cli/src/main/java/org/apache/tajo/cli/tsql/commands/ConnectDatabaseCommand.java — @@ -63,7 +63,7 @@ public void invoke(String[] cmd) throws Exception { @Override public String getUsage() { return ""; + return " [database_name] "; End diff – How about use ```DBNAME``` instead of ```database_name```? TSQL already uses ```DBNAME``` to print help descriptions with ```\help``` parameter.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user blrunner commented on the pull request:

        https://github.com/apache/tajo/pull/862#issuecomment-156965804

        If you add some unit test cases for this patch, it would be more better.

        Show
        githubbot ASF GitHub Bot added a comment - Github user blrunner commented on the pull request: https://github.com/apache/tajo/pull/862#issuecomment-156965804 If you add some unit test cases for this patch, it would be more better.
        Hide
        githubbot ASF GitHub Bot added a comment -

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

        https://github.com/apache/tajo/pull/862#discussion_r45013717

        — Diff: tajo-cli/src/main/java/org/apache/tajo/cli/tsql/commands/ConnectDatabaseCommand.java —
        @@ -63,7 +63,7 @@ public void invoke(String[] cmd) throws Exception {

        @Override
        public String getUsage() {

        • return "";
          + return "[database_name]";
            • End diff –

        I write ```database_name``` refer to ```DescTableCommand::getUsage()```. In ```DescTableCommand::getUsage()```, return ```[table_name]``` in spite of ```[TBNAME]``` with ```\help``` . You mean that editting other ```getUsage()``` is needed?

        Show
        githubbot ASF GitHub Bot added a comment - Github user dkhwangbo commented on a diff in the pull request: https://github.com/apache/tajo/pull/862#discussion_r45013717 — Diff: tajo-cli/src/main/java/org/apache/tajo/cli/tsql/commands/ConnectDatabaseCommand.java — @@ -63,7 +63,7 @@ public void invoke(String[] cmd) throws Exception { @Override public String getUsage() { return ""; + return " [database_name] "; End diff – I write ```database_name``` refer to ```DescTableCommand::getUsage()```. In ```DescTableCommand::getUsage()```, return ``` [table_name] ``` in spite of ``` [TBNAME] ``` with ```\help``` . You mean that editting other ```getUsage()``` is needed?
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user dkhwangbo commented on the pull request:

        https://github.com/apache/tajo/pull/862#issuecomment-157240753

        @blrunner Thanks for your review. I found another bug that ```getUsage()``` has no usage in whole TAJO, so I have no idea about making unit test cases. ```TajoShellCommand::printHelp()``` needs ```getUsage()``` but It's omitted. After resolve this issue in another ticket, I add unit test cases relate of this PR using shell command like ```TestTajoCli::testTimeZoneSessionVars1()```.

        Show
        githubbot ASF GitHub Bot added a comment - Github user dkhwangbo commented on the pull request: https://github.com/apache/tajo/pull/862#issuecomment-157240753 @blrunner Thanks for your review. I found another bug that ```getUsage()``` has no usage in whole TAJO, so I have no idea about making unit test cases. ```TajoShellCommand::printHelp()``` needs ```getUsage()``` but It's omitted. After resolve this issue in another ticket, I add unit test cases relate of this PR using shell command like ```TestTajoCli::testTimeZoneSessionVars1()```.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user blrunner commented on the pull request:

        https://github.com/apache/tajo/pull/862#issuecomment-159116854

        I know you've finished the issue which is involved with this PR. Could you add an unit test case?

        Show
        githubbot ASF GitHub Bot added a comment - Github user blrunner commented on the pull request: https://github.com/apache/tajo/pull/862#issuecomment-159116854 I know you've finished the issue which is involved with this PR. Could you add an unit test case?
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user dkhwangbo commented on the pull request:

        https://github.com/apache/tajo/pull/862#issuecomment-159118760

        @blrunner Oh, thanks for your remind. I'll add unit soon.

        Show
        githubbot ASF GitHub Bot added a comment - Github user dkhwangbo commented on the pull request: https://github.com/apache/tajo/pull/862#issuecomment-159118760 @blrunner Oh, thanks for your remind. I'll add unit soon.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user blrunner commented on the pull request:

        https://github.com/apache/tajo/pull/862#issuecomment-161533532

        +1

        LGTM!

        Show
        githubbot ASF GitHub Bot added a comment - Github user blrunner commented on the pull request: https://github.com/apache/tajo/pull/862#issuecomment-161533532 +1 LGTM!
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user asfgit closed the pull request at:

        https://github.com/apache/tajo/pull/862

        Show
        githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/tajo/pull/862
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Tajo-master-CODEGEN-build #616 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/616/)
        TAJO-1979: Usage of some TajoShellCommand is omitted. (blrunner: rev f775d7d29d2a7cc5fa7a8e9753548006fac8719f)

        • CHANGES
        • tajo-cli/src/main/java/org/apache/tajo/cli/tsql/commands/ConnectDatabaseCommand.java
        • tajo-core-tests/src/test/java/org/apache/tajo/cli/tsql/TestTajoCli.java
        • tajo-cli/src/main/java/org/apache/tajo/cli/tsql/commands/SetCommand.java
        • tajo-cli/src/main/java/org/apache/tajo/cli/tsql/commands/UnsetCommand.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Tajo-master-CODEGEN-build #616 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/616/ ) TAJO-1979 : Usage of some TajoShellCommand is omitted. (blrunner: rev f775d7d29d2a7cc5fa7a8e9753548006fac8719f) CHANGES tajo-cli/src/main/java/org/apache/tajo/cli/tsql/commands/ConnectDatabaseCommand.java tajo-core-tests/src/test/java/org/apache/tajo/cli/tsql/TestTajoCli.java tajo-cli/src/main/java/org/apache/tajo/cli/tsql/commands/SetCommand.java tajo-cli/src/main/java/org/apache/tajo/cli/tsql/commands/UnsetCommand.java
        Hide
        blrunner Jaehwa Jung added a comment -

        Committed to the master branch and 0.11.1 branch.

        Show
        blrunner Jaehwa Jung added a comment - Committed to the master branch and 0.11.1 branch.
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Tajo-master-build #1007 (See https://builds.apache.org/job/Tajo-master-build/1007/)
        TAJO-1979: Usage of some TajoShellCommand is omitted. (blrunner: rev f775d7d29d2a7cc5fa7a8e9753548006fac8719f)

        • CHANGES
        • tajo-cli/src/main/java/org/apache/tajo/cli/tsql/commands/UnsetCommand.java
        • tajo-cli/src/main/java/org/apache/tajo/cli/tsql/commands/SetCommand.java
        • tajo-core-tests/src/test/java/org/apache/tajo/cli/tsql/TestTajoCli.java
        • tajo-cli/src/main/java/org/apache/tajo/cli/tsql/commands/ConnectDatabaseCommand.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Tajo-master-build #1007 (See https://builds.apache.org/job/Tajo-master-build/1007/ ) TAJO-1979 : Usage of some TajoShellCommand is omitted. (blrunner: rev f775d7d29d2a7cc5fa7a8e9753548006fac8719f) CHANGES tajo-cli/src/main/java/org/apache/tajo/cli/tsql/commands/UnsetCommand.java tajo-cli/src/main/java/org/apache/tajo/cli/tsql/commands/SetCommand.java tajo-core-tests/src/test/java/org/apache/tajo/cli/tsql/TestTajoCli.java tajo-cli/src/main/java/org/apache/tajo/cli/tsql/commands/ConnectDatabaseCommand.java
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Tajo-0.11.1-build #126 (See https://builds.apache.org/job/Tajo-0.11.1-build/126/)
        TAJO-1979: Usage of some TajoShellCommand is omitted. (blrunner: rev e63721eb411820e1173e44ed823c18e6789496be)

        • tajo-core-tests/src/test/java/org/apache/tajo/cli/tsql/TestTajoCli.java
        • tajo-cli/src/main/java/org/apache/tajo/cli/tsql/commands/ConnectDatabaseCommand.java
        • tajo-cli/src/main/java/org/apache/tajo/cli/tsql/commands/UnsetCommand.java
        • CHANGES
        • tajo-cli/src/main/java/org/apache/tajo/cli/tsql/commands/SetCommand.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Tajo-0.11.1-build #126 (See https://builds.apache.org/job/Tajo-0.11.1-build/126/ ) TAJO-1979 : Usage of some TajoShellCommand is omitted. (blrunner: rev e63721eb411820e1173e44ed823c18e6789496be) tajo-core-tests/src/test/java/org/apache/tajo/cli/tsql/TestTajoCli.java tajo-cli/src/main/java/org/apache/tajo/cli/tsql/commands/ConnectDatabaseCommand.java tajo-cli/src/main/java/org/apache/tajo/cli/tsql/commands/UnsetCommand.java CHANGES tajo-cli/src/main/java/org/apache/tajo/cli/tsql/commands/SetCommand.java

          People

          • Assignee:
            dkhwangbo Dongkyu Hwangbo
            Reporter:
            dkhwangbo Dongkyu Hwangbo
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development