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