Proposed BREAKING CHANGES to NLog

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

Proposed BREAKING CHANGES to NLog

Jarek Kowalski
Administrator
Hi!

I'd like to hear your opinion on two potentially breaking changes I'd like
to make to NLog before 1.0. They will be included in NLog-1.0 RC2.

CHANGE #1:

Add NLog.ILayout interface and make NLog.Layout implement it and change
CompiledLayout property of target to become ILayout instead of Layout. Other
layouts used (such as FileTarget.FileName would be left unchanged).

REASON FOR CHANGE:

There are cases where you need to emit strings formatted in some particular
manner, such as:

* XML (you need to escape some special characters)
* CSV (you need to quote some strings)
* HTML with coloring
* ANSI coloring on Unix
* TeX, PostScript, RTF, ROT-13, Base64, UUENCODE...

The formatting seems to be independent of the target used (for example you
can send CSV-formatted text over the Network or write ANSI-colored text to a
File).

The plan is to implement these layout types after 1.0 (this would include
LayoutFactory, LayoutAttribute, configurator and NLog.xsd updates as usual),
but to enable this we need to make this change now.

CHANGE #2:

Add NLog.TargetBase class that will be a base for all target types. It will
NOT have any layout information, in particular Layout and CompiledLayout
won't be available.

REASON FOR CHANGE:

There are some types of targets which ignore layouts (they are Database,
MethodCall, WebService, all wrappers and compound targets and more). Today
they inherit from Target which gives them the "Layout" and "CompiledLayout"
properties which they just ignore. Setting these properties makes no sense
and they "spoil" the API. I believe that we should have:

class TargetBase
{
    string Name { get; set; }
}

class Target : TargetBase
{
    string Layout { get; set; }
    ILayout CompiledLayout { get; set; }
}

Alternatively we could have:

class Target
{
}

and

class TargetWithLayout : Target
{
}

The latter API looks nicer than the former one, but would potentially break
code that's based on the examples we provide on the web. TargetBase/Target
approach seems to break (a lot of) code which relied on things being Target
which have now became TargetBase.

But should we care? Pre-1.0 time is meant for such changes.

The proposed patch (which unfortunately implements TargetBase/Target) is
attached. I'd like to hear your comments before I commit it. It also updates
the NLogViewer target which can now be now simplified because of the new
layouts.

Regards,

Jarek

a.diff (58K) Download Attachment
NLog Blog