[logback-dev] Logback Beagle: first impression

[ceki]

Comments inline.

On 13.07.2012 22:53, Christian Trutz wrote:
> Hi together,

He, he.

> here my comments on Logback Beagle Release 1.0.0:
>
> 1. "git tag" missing for version 1.0.0

done

> 2. convert context menu (MenuBuilder,OnMenuSelectionAction) to Eclipse commands

Let's talk about item 2 at a later stage.

> 3. migrate functionality from TableMediator to BeagleView and remove
> TableMediator

For the time being, I am keen on being able to launch tests without a 
view. Moving TableMediator  functionality to BeagleView would remove 
that capability.

> 4. Introduce LabelProvider and ContentProvider for virtual table (BeagleView)
> 5. ITableItemStub#getText() and #getImage() --> LabelProvider

See my answer to item 7 below.

> 6. Remove VistaManager

Indeed. It's unused at the moment. It was introduced in order to be able 
to log to different tables. We currently do not offer this feature. So 
we can remove VistaManager.

> 7. Remove ITableItemStubs (incl. subclasses) --> ContentProvider

I looked at ContentProvider and LabelProvider briefly a while ago. These 
work with JFace tables (TableViewer) whereas logback-beagle uses a 
plain-old SWT Table at the moment.

Refactoring the code to use ContentProvider instead of ITableItemStubs 
would be a *big* undertaking. Wouldn't it? Nevertheless, I would be 
happy if you took a shot at it.

In the mean time, I'll read up on ContentProvider.

> 8. remove System.out.println (many occurences in source code)

Sure.

> 9. ResourceUtil#IMAGE_MAP -> use instead ImageRegistry (see also
> http://www.eclipse.org/articles/Article-Using%20Images%20In%20Eclipse/Using%20Images%20In%20Eclipse.html)

I assume you's use a ImageDescriptor. Cool. I suggest we start with this 
last item since it appears to be the easiest non-trivial item.

-- 
Ceki
http://tinyurl.com/proLogback