[GitHub] ant-ivy pull request #71: Ivy main/standalone: Patch to include 'makepom' fu...

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] ant-ivy pull request #71: Ivy main/standalone: Patch to include 'makepom' fu...

asfgit
GitHub user aanno opened a pull request:

    https://github.com/apache/ant-ivy/pull/71

    Ivy main/standalone: Patch to include 'makepom' function

    Hello,
   
    I added the pomfile option to main/standalone. This allows creating an (maven) pom file from outside an ant task.
   
    Example of use:
   
    ```sh
    $ pwd
    ~/.ivy2/cache/org.typelevel/cats-core_2.11
    $ java -jar ~/scm/github/ant-ivy/build/artifact/org.apache.ivy_2.5.0.alpha_20180327212209.jar -ivy ivy-1.0.1.xml -pomfile cats-core.xml
    $ ls
    cats-core-2.11.xml  ivy-1.0.1.xml  ivy-1.0.1.xml.original  ivydata-1.0.1.properties  jars  srcs
    ```
    Feedback is welcome. What should I do to get this patch into mainline?
   
    Kind regards,
   
    aanno

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

    $ git pull https://github.com/aanno/ant-ivy feature/aanno-main-pomfile

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

    https://github.com/apache/ant-ivy/pull/71.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 #71
   
----
commit 5e814ad7a84ffb2a9d4df72fc801e9b8d6aa2f64
Author: Thomas Pasch <thomas.pasch@...>
Date:   2018-03-27T19:17:01Z

    Added pomfile option to main/standalone

----


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant-ivy pull request #71: Ivy main/standalone: Patch to include 'makepom' fu...

asfgit
Github user jaikiran commented on a diff in the pull request:

    https://github.com/apache/ant-ivy/pull/71#discussion_r177694759
 
    --- Diff: src/java/org/apache/ivy/Main.java ---
    @@ -199,6 +201,10 @@ static CommandLineParser getParser() {
                         new OptionBuilder("cp").arg("cp")
                                 .description("extra classpath to use when launching process").create())
     
    +                .addCategory("maven compatibility options")
    +                .addOption(new OptionBuilder("pomfile").arg("pomfile").countArgs(false)
    --- End diff --
   
    A small suggestion - can you change this to something like:
    ```
    new OptionBuilder("makepom").arg("pomfile")....
    ```


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant-ivy pull request #71: Ivy main/standalone: Patch to include 'makepom' fu...

asfgit
In reply to this post by asfgit
Github user jaikiran commented on a diff in the pull request:

    https://github.com/apache/ant-ivy/pull/71#discussion_r177694862
 
    --- Diff: src/java/org/apache/ivy/Main.java ---
    @@ -199,6 +201,10 @@ static CommandLineParser getParser() {
                         new OptionBuilder("cp").arg("cp")
                                 .description("extra classpath to use when launching process").create())
     
    +                .addCategory("maven compatibility options")
    +                .addOption(new OptionBuilder("pomfile").arg("pomfile").countArgs(false)
    +                            .description("makepom as standalone tasks").create())
    --- End diff --
   
    I think the description should be a bit more clear and state that this generates a pom file for the resolved module.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant-ivy issue #71: Ivy main/standalone: Patch to include 'makepom' function

asfgit
In reply to this post by asfgit
Github user jaikiran commented on the issue:

    https://github.com/apache/ant-ivy/pull/71
 
    @aanno Thank you for the patch. This mostly looks fine. I have added some review comments. Can you please also introduce a test case for this? Something that tests that these command options are recognized and the pom file does get created.  There's a `MainTest` which you can refer to and add a new test method there.



---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant-ivy pull request #71: Ivy main/standalone: Patch to include 'makepom' fu...

asfgit
In reply to this post by asfgit
Github user twogee commented on a diff in the pull request:

    https://github.com/apache/ant-ivy/pull/71#discussion_r177696348
 
    --- Diff: src/java/org/apache/ivy/Main.java ---
    @@ -199,6 +201,10 @@ static CommandLineParser getParser() {
                         new OptionBuilder("cp").arg("cp")
                                 .description("extra classpath to use when launching process").create())
     
    +                .addCategory("maven compatibility options")
    +                .addOption(new OptionBuilder("pomfile").arg("pomfile").countArgs(false)
    +                            .description("makepom as standalone tasks").create())
    --- End diff --
   
    IMHO `pomfile` is confusing (cf use of `ivyfile` and `propertiesfile`). I'd suggest `writepom`or something like that.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant-ivy pull request #71: Ivy main/standalone: Patch to include 'makepom' fu...

asfgit
In reply to this post by asfgit
Github user twogee commented on a diff in the pull request:

    https://github.com/apache/ant-ivy/pull/71#discussion_r177857645
 
    --- Diff: src/java/org/apache/ivy/Main.java ---
    @@ -199,6 +201,10 @@ static CommandLineParser getParser() {
                         new OptionBuilder("cp").arg("cp")
                                 .description("extra classpath to use when launching process").create())
     
    +                .addCategory("maven compatibility options")
    +                .addOption(new OptionBuilder("pomfile").arg("pomfile").countArgs(false)
    +                            .description("makepom as standalone tasks").create())
    --- End diff --
   
    On the second thoughts, why not calling the option `makepom` 😉 ?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant-ivy issue #71: Ivy main/standalone: Patch to include 'makepom' function

asfgit
In reply to this post by asfgit
Github user jaikiran commented on the issue:

    https://github.com/apache/ant-ivy/pull/71
 
    @aanno, thank you for this PR. I have gone ahead and included your patch with minor modifications to upstream and also included a test along with it. I have also included your name in our release notes. Since I couldn't find your full name in your github profile, I used `aanno`, if you prefer to have your full name included in the release notes, please let me know and I'll update it accordingly.



---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant-ivy issue #71: Ivy main/standalone: Patch to include 'makepom' function

asfgit
In reply to this post by asfgit
Github user twogee commented on the issue:

    https://github.com/apache/ant-ivy/pull/71
 
    @aanno could you please rebase and push the branch in order to close the PR?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]