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

2020-07-13 Thread Alexey Semenyuk
+1 - Alexey On 7/10/2020 6:29 PM, Andy Herrick wrote: looks good. /Andy On 7/9/2020 12:02 AM, alexander.matv...@oracle.com wrote: Hi Alexey, http://cr.openjdk.java.net/~almatvee/8248261/webrev.01/ - Added fatalError() to log fatal errors without timestamp. - Added missing timestamp to

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

2020-07-10 Thread Andy Herrick
looks good. /Andy On 7/9/2020 12:02 AM, alexander.matv...@oracle.com wrote: 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

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

2020-07-08 Thread alexander . matveev
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

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

2020-07-08 Thread Alexey Semenyuk
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()

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

2020-07-08 Thread Andy Herrick
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

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

2020-07-07 Thread Alexey Semenyuk
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

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

2020-07-07 Thread Andy Herrick
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.matv...@oracle.com wrote: Hi Andy,

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

2020-07-07 Thread alexander . matveev
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:

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

2020-07-07 Thread Andy Herrick
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

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

2020-07-06 Thread alexander . matveev
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