RFR: 8248261: Add timestamps to jpackage and jpackage tests verbose output
Alexey Semenyuk
alexey.semenyuk at oracle.com
Wed Jul 8 16:34:07 UTC 2020
I still think it would be good to create dedicated method for final
error reporting in Main and Arguments classes without timestamps.
- Alexey
On 7/8/2020 9:59 AM, Andy Herrick wrote:
> 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