[rfc][icedtea-web] refactored logging

Omair Majid omajid at redhat.com
Thu Sep 12 10:09:53 PDT 2013


Hi Jiri,

On 09/12/2013 06:27 AM, Jiri Vanek wrote:
> On 09/11/2013 06:29 PM, Pavel Tisnovsky wrote:
>> ----- Omair Majid <omajid at redhat.com> wrote:
>>> I am mostly interested in this patch as a user (especially if some else
>>> has agreed to a full review of this giant patch). But it caught my
>>> interest so I have a few questions.
>>>
>>> First, please don't abuse 'refactoring'. It's defined here:
>>> http://refactoring.com/. This patch is a change (probably a good one);
>>> it's not (just) a refactor.
> 
> It is refactoring - the other changes are consequence - eg skeleton
> methods into which the ex.prrintstacktrace or system.out/err  are
> refactroed.

You are correct. I skimmed over the patch quickly and made a (serious)
mistake. Apologies.

>>> Are you saying the old logger is broken? Can you elaborate?
>>>
> 
> Not broken, as working, but broken as not doing its job - it was logging
> really only few, minor exceptions.  User needs to be told about it (or
> found on his own how to enable it and where to found the logs)
>

Are those exceptions worth logging? (or even worth stopping and showing
an error to the user?)

>>> I am probably missing something, but why isn't ItwLogger a
>>> java.util.logging.Logger?
> 
> Why it should be? /me being ironic, but have not found a real  answer to
> this question

Same reason that it's better to use something that already exists rather
than re-implementing it: it has been tested more (by more people), it
has an API that more people are familiar with, and it should be more
capable and generic than something we specifically develop for icedtea-web.

That last point may be a downside too - the API may not be exactly what
we need.

I am looking forward to an updated patch. Please do include unit tests
too (as much as possible).

Thanks,
Omair

-- 
PGP Key: 66484681 (http://pgp.mit.edu/)
Fingerprint = F072 555B 0A17 3957 4E95  0056 F286 F14F 6648 4681



More information about the distro-pkg-dev mailing list