RFR: 8248261: Add timestamps to jpackage and jpackage tests verbose output
alexander.matveev at oracle.com
alexander.matveev at oracle.com
Thu Jul 9 04:02:03 UTC 2020
Hi Alexey,
http://cr.openjdk.java.net/~almatvee/8248261/webrev.01/
- Added fatalError() to log fatal errors without timestamp.
- Added missing timestamp to Log.verbose(Throwable t).
Thanks,
Alexander
On 7/8/20 9:34 AM, Alexey Semenyuk wrote:
> 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