RFR: JDK-8230521: rename --output/-o option and add default value (".")
Alexander Matveev
alexander.matveev at oracle.com
Sat Sep 14 00:20:35 UTC 2019
http://cr.openjdk.java.net/~almatvee/8230521/webrev.01/
- Simplified setting default value for destination folder.
- Undo renaming of output variables to dest.
- Modified help messages as per Andy suggestion.
- Removed obsolete tests as per Alexey suggestion.
Thanks,
Alexander
On 9/12/2019 8:16 PM, Alexey Semenyuk wrote:
>
>
> 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