RFR: JDK-8230521: rename --output/-o option and add default value (".")

Alexey Semenyuk alexey.semenyuk at oracle.com
Fri Sep 13 03:16:08 UTC 2019


On 9/12/2019 10:47 PM, Alexey Semenyuk wrote:
> http://cr.openjdk.java.net/~almatvee/8230521/webrev.00/src/jdk.jpackage/share/classes/jdk/jpackage/internal/Arguments.java.sdiff.html: 
>
> ---
> 126         Path destPath = Paths.get(".").toAbsolutePath();
>  127         if (destPath.getFileName().toString().equals(".")) {
>  128             dest = destPath.getParent().toString();
>  129         } else {
>  130             dest = destPath.toString();
>  131         }
> ---
> Are you trying to get rid of trailing "." path component in the above 
> code fragment? If yes, then this can be simplified as follows:
> ---
> Path destPath = Paths.get(".").normalize().toAbsolutePath();
> ---
Or even simpler:
---
Path destPath = Paths.get("").toAbsolutePath();
---

- Alexey
>
> In general I don't like the idea of renaming variables and method 
> names from `output` to `dest`. This doesn't add much value to the code 
> and is incomplete and thus confusing (e.g.: in all the tests OUTPUT 
> variable remains unnamed).
> I suggest to change only `--output` to `--dest` tokens in the code.
>
> Slightly unrelated thing. macOS tests
> ---
> test/jdk/tools/jpackage/macosx/base/Base.java
> test/jdk/tools/jpackage/macosx/base/FileAssociationsBase.java
> test/jdk/tools/jpackage/macosx/base/InstallDirBase.java
> test/jdk/tools/jpackage/macosx/base/LicenseBase.java
> ---
> duplicate corresponding shared tests that have been added in the patch 
> for https://bugs.openjdk.java.net/browse/JDK-8230726. I didn't touch 
> them as I didn't have MAC to try the replacements. Could you please 
> just drop these obsolete tests and also run the new tests to make sure 
> they work as expected. I mean not just run the tests to create package 
> bundles, but also install these bundles and run the tests in a mode 
> when they verify packages are properly installed.
>
> - Alexey
>
> On 9/12/2019 9:34 PM, Alexander Matveev wrote:
>> Please review the jpackage fix for bug [1] at [2].
>>
>> This is a fix for the JDK-8200758-branch branch of the open sandbox 
>> repository (jpackage).
>>
>> - Renamed "--output" to "--dest" and made it optional with default to 
>> ".".
>>
>> [1] https://bugs.openjdk.java.net/browse/JDK-8230521
>>
>> [2] http://cr.openjdk.java.net/~almatvee/8230629/webrev.00/
>>
>> Thanks,
>> Alexander
>



More information about the core-libs-dev mailing list