You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@groovy.apache.org by remkop <gi...@git.apache.org> on 2018/05/04 07:37:48 UTC

[GitHub] groovy pull request #694: GROOVY-8567 Migrate Groovyc to picocli

GitHub user remkop opened a pull request:

    https://github.com/apache/groovy/pull/694

    GROOVY-8567 Migrate Groovyc to picocli

    GROOVY-8567 Migrate Groovyc to picocli
    
    See comments in the JIRA ticket.

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

    $ git pull https://github.com/remkop/groovy master

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

    https://github.com/apache/groovy/pull/694.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 #694
    
----
commit f115ef292b859f3035a2894d438679e1d39268ad
Author: Remko Popma <re...@...>
Date:   2018-04-29T23:39:40Z

    GROOVY-8520 CliBuilder groovydoc improvements

commit 10a7a3c6dc8a4974a4ed7fd1b35adabcd079eaa9
Author: Remko Popma <re...@...>
Date:   2018-05-01T21:33:42Z

    Merge remote-tracking branch 'upstream/master'

commit de3c3a8bcd04a4c05cbf605100f1a029417ccb17
Author: Remko Popma <re...@...>
Date:   2018-05-03T13:53:26Z

    Merge remote-tracking branch 'upstream/master'

commit c1fd6cf55b5c59cf08a5efdaabc91b9656296690
Author: Remko Popma <re...@...>
Date:   2018-05-04T07:35:05Z

    Merge remote-tracking branch 'upstream/master'

commit 4679fc5ee2ec823e22c3d5375f651c255dbd0bed
Author: Remko Popma <re...@...>
Date:   2018-05-04T07:35:42Z

    GROOVY-8567 Migrate Groovyc to picocli

----


---

[GitHub] groovy pull request #694: GROOVY-8567 Migrate Groovyc to picocli

Posted by paulk-asert <gi...@git.apache.org>.
Github user paulk-asert commented on a diff in the pull request:

    https://github.com/apache/groovy/pull/694#discussion_r186015291
  
    --- Diff: src/main/java/org/codehaus/groovy/tools/FileSystemCompiler.java ---
    @@ -77,18 +73,6 @@ public void compile(File[] files) throws Exception {
             unit.compile();
         }
     
    -    public static void displayHelp(final Options options) {
    -        final HelpFormatter formatter = new HelpFormatter();
    -        formatter.printHelp(80, "groovyc [options] <source-files>", "options:", options, "");
    -    }
    -
    -    public static void displayVersion() {
    --- End diff --
    
    For binary compatibility we'd want to keep but deprecate this method. Ideally, you'd change implementation to point to new details. If that's not possible you can throw a DeprecationException.


---

[GitHub] groovy pull request #694: GROOVY-8567 Migrate Groovyc to picocli

Posted by remkop <gi...@git.apache.org>.
Github user remkop commented on a diff in the pull request:

    https://github.com/apache/groovy/pull/694#discussion_r186292414
  
    --- Diff: src/main/java/org/codehaus/groovy/tools/FileSystemCompiler.java ---
    @@ -77,18 +73,6 @@ public void compile(File[] files) throws Exception {
             unit.compile();
         }
     
    -    public static void displayHelp(final Options options) {
    --- End diff --
    
    I've reinstated the `displayHelp(org.apache.commons.cli.Options)` method and made it deprecated. This can be merged with the 2_5_X branch as is, and the method (and associated imports) can be removed for 2_6_X and master.


---

[GitHub] groovy pull request #694: GROOVY-8567 Migrate Groovyc to picocli

Posted by remkop <gi...@git.apache.org>.
Github user remkop commented on a diff in the pull request:

    https://github.com/apache/groovy/pull/694#discussion_r186035996
  
    --- Diff: src/main/java/org/codehaus/groovy/tools/FileSystemCompiler.java ---
    @@ -77,18 +73,6 @@ public void compile(File[] files) throws Exception {
             unit.compile();
         }
     
    -    public static void displayHelp(final Options options) {
    --- End diff --
    
    Okay. Note that the Options class is no longer used anywhere in `FileSystemCompiler` so this would be purely for binary compatibility.
    
    How do you want to do this? I'm working on master, so the method is removed in the PR. Do you want a separate PR for 2_5_X with the method reinstated or will you manually reinstate on the 2_5_X branch when cherrypicking?


---

[GitHub] groovy pull request #694: GROOVY-8567 Migrate Groovyc to picocli

Posted by remkop <gi...@git.apache.org>.
Github user remkop closed the pull request at:

    https://github.com/apache/groovy/pull/694


---

[GitHub] groovy pull request #694: GROOVY-8567 Migrate Groovyc to picocli

Posted by paulk-asert <gi...@git.apache.org>.
Github user paulk-asert commented on a diff in the pull request:

    https://github.com/apache/groovy/pull/694#discussion_r186015829
  
    --- Diff: src/main/java/org/codehaus/groovy/tools/FileSystemCompiler.java ---
    @@ -77,18 +73,6 @@ public void compile(File[] files) throws Exception {
             unit.compile();
         }
     
    -    public static void displayHelp(final Options options) {
    --- End diff --
    
    Not 100% sure on this one. I think we might need to keep it and mark as deprecated in 2_5_X and then remove in 2_6_X and master. That means we won't be able to remove commons cli fully just yet.


---

[GitHub] groovy pull request #694: GROOVY-8567 Migrate Groovyc to picocli

Posted by remkop <gi...@git.apache.org>.
Github user remkop commented on a diff in the pull request:

    https://github.com/apache/groovy/pull/694#discussion_r186035195
  
    --- Diff: src/main/java/org/codehaus/groovy/tools/FileSystemCompiler.java ---
    @@ -77,18 +73,6 @@ public void compile(File[] files) throws Exception {
             unit.compile();
         }
     
    -    public static void displayHelp(final Options options) {
    -        final HelpFormatter formatter = new HelpFormatter();
    -        formatter.printHelp(80, "groovyc [options] <source-files>", "options:", options, "");
    -    }
    -
    -    public static void displayVersion() {
    --- End diff --
    
    Done in commit 3900e3b.


---