OK.

-- Kevin


On 11/9/2019 11:46 AM, Alexey Semenyuk wrote:
Kevin,

My point was to add consistency. We used to have class names in camel case and pascal case (className, ClassName) and this didn't look right for me. I agree ClassName, `mypkg` is more appropriate. I'm neutral about myJar.jar and MyJar.jar.

- Alexey

On 11/9/2019 12:37 AM, Kevin Rushforth wrote:
I disagree with a couple of these changes, but they can be fixed in a subsequent pass, since you've already pushed the changes.

By convention, class names are camel case with leading upper-case letter, so ClassName expresses that better than className. Similarly, MyJar.jar seems better to me than myJar.jar. By convention, package names don't have upper-case letters at all, so package.name or `mypkg` might be better than packageName.

-- Kevin

On 11/9/2019 2:50 AM, Alexey Semenyuk wrote:
Looks good.

- Alexey

On 11/8/2019 4:31 PM, Andy Herrick wrote:
revised [3] as per below suggestions.

[3] http://cr.openjdk.java.net/~herrick/8233591/webrev.03

/Andy


On 11/8/2019 4:07 PM, Alexey Semenyuk wrote:
This is minor and not directly related to this change, but looks like we have inconsistency in names in Sample usages. Names used: modulePath, ModulePath, moduleName, className, moduleName/ClassName, moduleName/className, package.ClassName, inputdir, MyJar.jar, appRuntimeImage, name, <app image dir>. I'd suggest the following changes if we are touching this section of help anyways:
package.ClassName -> packageName.className
MyJar.jar -> myJar.jar
inputdir -> inputDir
ModulePath -> modulePath
moduleName/ClassName -> moduleName/className
<app image dir> -> appImageDir

- Alexey

On 11/8/2019 3:41 PM, Andy Herrick wrote:
Please review the revised jpackage fix for bug [1] at [2].

This is a fix for the JDK-8200758-branch branch of the open sandbox repository (jpackage).

This fix (webrev.02) was revised after feedback from webrev.01.

[1] https://bugs.openjdk.java.net/browse/JDK-8233591

[2] http://cr.openjdk.java.net/~herrick/8233591/webrev.02/

/Andy






Reply via email to