RFR: 8248261: Add timestamps to jpackage and jpackage tests verbose output
Alexey Semenyuk
alexey.semenyuk at oracle.com
Tue Jul 7 23:23:03 UTC 2020
Alexander,
We probably need a clean separation of logging from error reporting as
Andy pointed out in his comment. I don't think having timestamps only in
verbose output is a good idea.
Besides at some point we might will use java.util.logging.Logger instead
of jdk.incubator.jpackage.internal.Log and having this clean separation
would make this transition easier.
- Alexey
On 7/7/2020 6:51 PM, alexander.matveev at oracle.com wrote:
> Hi Andy,
>
> Timestamps for error message without verbose output are meaningless in
> my opinion. This is why I did not add them. Also, in some cases output
> does not look right. For example when timestamp is added to error
> message always:
> jpackage --someoption
> WARNING: Using incubator modules: jdk.incubator.jpackage
> [15:49:23.798] Error: Invalid Option: [--someoption]
>
> In example above I do not see any point to have timestamp.
>
> Thanks,
> Alexander
>
> On 7/7/20 6:18 AM, Andy Herrick wrote:
>> All looks good, except maybe Log.error().
>>
>> I think Log.error() should have the same output whether verbose or
>> not, adding timestamp to the message only if verbose does not look
>> right.
>>
>> Problem is that Log.error() is used for two fundamentally different
>> purposes:
>>
>> 1.) by Main and Arguments to display the final error message when
>> jpackage is failing.
>>
>> 2.) by everyone else to display a serious warning message when
>> jpackage is continuing (sometimes failing further on, and sometimes
>> not).
>>
>> I wouldn't mind adding timestamps only when verbose in case (2), but
>> I don't think that's appropriate for case (1).
>>
>> /ANdy
>>
>> On 7/6/2020 8:27 PM, alexander.matveev at oracle.com wrote:
>>> Please review the jpackage fix for bug [1] at [2].
>>>
>>> Added timestamp to verbose and test output in form of [HH:mm:ss.SSS].
>>>
>>> [1] https://bugs.openjdk.java.net/browse/JDK-8248261
>>> [2] http://cr.openjdk.java.net/~almatvee/8248261/webrev.00/
>>>
>>> Thanks,
>>> Alexander
>
More information about the core-libs-dev
mailing list