RFR: 8248261: Add timestamps to jpackage and jpackage tests verbose output

Andy Herrick andy.herrick at oracle.com
Wed Jul 8 13:59:46 UTC 2020


After further discussion, I approve this change as is.

The main fatal error in Arguments.processArguments() only calls 
Log.error if not verbose , so guarding it from having a timestamp when 
verbose is not an issue.

/Andy

On 7/7/2020 7:20 PM, Andy Herrick wrote:
> I agree - but I would rather then that Log.error() never show timestamp.
>
> It would be better still if we could differentiate fatal error 
> messages from warning messages - right now we are using Log.error for 
> both.
>
> /Andy
>
>
> 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