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

[twogee]

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: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

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

[twogee]

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: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

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

[jaikiran]

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: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

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

[jaikiran]

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: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

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

[jaikiran]

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: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org