[Feature] Serilog w/ NLog support {WIP} #61

ruba1987 · 2014-06-20T19:54:07Z

Support for using serilog piping to NLog

jonnii · 2014-06-21T15:22:03Z

woot 👍

jonorossi · 2014-06-23T07:10:44Z

src/Castle.Services.Logging.SerilogIntegration/SerilogFactory.cs

+                .MinimumLevel.Is(LogEventLevel.Debug)
+                .WriteTo.ColoredConsole()
+                .WriteTo.NLog()
+                .CreateLogger();


I haven't had a chance to use Serilog yet, however is there a reason we'd want to hardcode Castle Core to using the NLog sink rather than allow the consumer to set their own configuration like the log4net and NLog integration?

ruba1987 · 2014-06-30T21:10:57Z

So now that this is refactored it no longer requires serilog/serilog#153.

I added some tests and cleaned up the code a bit here and there.

What do you think?

jonnii · 2014-07-01T00:46:49Z

src/Castle.Services.Logging.SerilogIntegration/packages.config

+<packages>
+  <package id="Castle.Core" version="3.3.0" targetFramework="net45" />
+  <package id="NLog" version="3.1.0.0" targetFramework="net40" requireReinstallation="True" />
+  <package id="Serilog" version="1.3.33" targetFramework="net45" />


maybe loosen this up to 1.3, or even 1.x?

jonorossi · 2014-07-01T01:50:50Z

Thanks @ruba1987, sorry for so many comments, but I've added a few more to the updated diff.

ruba1987 · 2014-07-01T14:57:58Z

Thanks @jonorossi @jonnii for the feedback. I think I have everything updated now.

jonorossi · 2014-07-02T00:58:14Z

@ruba1987 Just a few more comments:

You've just now added some more NuGet into the solution file for a ".nuget" directory.
That #ifdef on src/Castle.Core.Tests/Properties/Settings.Designer.cs is still removed.
Seeing as Serilog only supports .NET 4.5 (that's the only DLL you've added), I think we will need to do something to remove those build targets, and conditionally compile the new code in Castle.Core.Tests.

ruba1987 · 2014-07-02T01:24:48Z

src/Castle.Core.Tests/Properties/Settings.Designer.cs

@@ -9,9 +9,10 @@
 //------------------------------------------------------------------------------

 namespace CastleTests.Properties {
-#if !MONO && !SILVERLIGHT
+#if !MONO && !SILVERLIGHT    


It's here, just on a diff line 12

Ah, thanks. Whitespace changes.

Oh, I missed that. I will fix that then too.

ruba1987 · 2014-07-02T01:27:55Z

@jonorossi Let me see what serilog supports for other versions. If I can't find anything I will figure something out to do that. Do you have any suggestions?

And I will remove the .nuget stuff from the .sln file.... that stuff just keeps turning up everywhere... :-(

jonorossi · 2014-07-02T01:42:52Z

If Serilog only supports .NET 4.5, that isn't a big problem. We should be able to remove the compilation targets for the unsupported versions in the csproj, and use the DOTNET45 define to conditionally compile the unit test code out. I'm not sure if we'll have any problems building the NuGet packages, but we can work that out when we get to it after we merge.

http://builds.castleproject.org/viewType.html?buildTypeId=Core_Pack

jonorossi · 2014-07-02T11:41:34Z

@ruba1987 Before I forget could you please squish all of your commits into a single commit with a descriptive commit message when you are finished. Thanks.

ruba1987 · 2014-07-02T12:35:45Z

I will deff rebase and squash it before you merge it. I was just waiting
until you guys are happy with it first.
On Jul 2, 2014 7:41 AM, "Jonathon Rossi" notifications@github.com wrote:

@ruba1987 https://github.com/ruba1987 Before I forget could you please
squish all of your commits into a single commit with a descriptive commit
message when you are finished. Thanks.

—
Reply to this email directly or view it on GitHub
#61 (comment).

jonorossi · 2014-07-02T12:51:54Z

Thanks. I've set up your branch in TeamCity so we can sort out any build issues with the unsupported framework versions. It has a VCS trigger so you don't need any permissions to start a build.

http://builds.castleproject.org/project.html?projectId=core_ruba1987_serilog

It looks like the Serilog integration project file still has the AssemblyInfo.cs file listed but not committed, however it is referencing the CommonAssemblyInfo.cs file from the root.

ruba1987 · 2014-07-02T17:07:54Z

yea, I noticed that. I'm actually a bit confused about that. It looks like the AssemblyInfo.cs files are generated but I don't know how to set that up. They are also listed in the .gitignore file so I would think they aren't supposed to be committed. How do I set that part up?

For now, I have added it to the repo to get the builds in TC working right and then once I know how to set that part up right I will change it.

ruba1987 · 2014-07-02T17:18:37Z

@jonorossi Ok, the latest version I pushed seems to build fine in all versions. I added Serilog support for 4.0 and 4.5.

Let me know if this looks good and I will squash it.

jonorossi · 2014-07-03T04:06:17Z

Castle.Core.sln

 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution
+		Debug|Any CPU = Debug|Any CPU


Could you remove the generic Debug and Release targets from the new C# project.

ruba1987 · 2014-07-03T15:00:22Z

@jonorossi looks like I squared everything away and now it builds just fine via TC for all builds.

jonorossi · 2014-07-04T01:08:28Z

@ruba1987 I feel really bad continuing to write comments on things that need to change, sorry. The AssemblyInfo.cs file is still listed as committed. Everything else looks good.

ruba1987 · 2014-07-04T01:59:33Z

Ah damn, I completely forgot to remove that. Sorry for the headache, I will
take care of it as soon as I can log on
On Jul 3, 2014 9:08 PM, "Jonathon Rossi" notifications@github.com wrote:

@ruba1987 https://github.com/ruba1987 I feel really bad continuing to
write comments on things that need to change, sorry. The AssemblyInfo.cs
file is still listed as committed. Everything else looks good.

—
Reply to this email directly or view it on GitHub
#61 (comment).

ruba1987 · 2014-07-06T13:49:52Z

@jonorossi Ok, I fixed that issue with the AssemblyInfo.cs file. I also rebased and squashed. let me know if there is anything else that needs to be done on this.

jonorossi · 2014-07-07T02:02:18Z

@ruba1987 Thanks, looks all good to me. Sorry this turned out to be painful.

Added support for Serilog.

ruba1987 · 2014-07-07T12:15:25Z

lol, no worries.

On Sun, Jul 6, 2014 at 10:03 PM, Jonathon Rossi notifications@github.com
wrote:

Merged #61 #61.

—
Reply to this email directly or view it on GitHub
#61 (comment).

ruba1987 · 2014-09-09T15:45:21Z

@jonorossi any chance you guys can push this to nuget? I would like to use the package

jonorossi · 2014-09-10T02:47:03Z

@kkozmic @hammett you guys are the owners of the Castle.Core and both existing logging integration packages. Did you want to push this new one?

jonorossi · 2014-09-10T06:44:51Z

@kkozmic do you think we should release Castle Core 3.3.1 with this new package?

kkozmic · 2014-09-10T06:46:02Z

Makes sense

sent from my phone
On 10 Sep 2014 16:44, "Jonathon Rossi" notifications@github.com wrote:

@kkozmic https://github.com/kkozmic do you think we should release
Castle Core 3.3.1 with this new package?

—
Reply to this email directly or view it on GitHub
#61 (comment).

jonorossi · 2014-09-10T13:33:03Z

@ruba1987 I've released 3.3.1 to nuget.org. Let me know how you go.

ruba1987 · 2014-09-10T15:01:43Z

@jonorossi perfect! It works great. Thanks.

jonorossi reviewed Jun 23, 2014
View reviewed changes

jonnii reviewed Jul 1, 2014
View reviewed changes

ruba1987 reviewed Jul 2, 2014
View reviewed changes

jonorossi reviewed Jul 3, 2014
View reviewed changes

Added support for serilog

2931ac3

jonorossi added a commit that referenced this pull request Jul 7, 2014

Merge pull request #61 from ruba1987/feature/serilog-support

b677193

Added support for Serilog.

jonorossi merged commit b677193 into castleproject:master Jul 7, 2014

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature] Serilog w/ NLog support {WIP} #61

[Feature] Serilog w/ NLog support {WIP} #61

ruba1987 commented Jun 20, 2014

jonnii commented Jun 21, 2014

jonorossi Jun 23, 2014

ruba1987 commented Jun 30, 2014

jonnii Jul 1, 2014

jonorossi commented Jul 1, 2014

ruba1987 commented Jul 1, 2014

jonorossi commented Jul 2, 2014

ruba1987 Jul 2, 2014

jonorossi Jul 2, 2014

ruba1987 Jul 2, 2014

ruba1987 commented Jul 2, 2014

jonorossi commented Jul 2, 2014

jonorossi commented Jul 2, 2014

ruba1987 commented Jul 2, 2014

jonorossi commented Jul 2, 2014

ruba1987 commented Jul 2, 2014

ruba1987 commented Jul 2, 2014

jonorossi Jul 3, 2014

ruba1987 commented Jul 3, 2014

jonorossi commented Jul 4, 2014

ruba1987 commented Jul 4, 2014

ruba1987 commented Jul 6, 2014

jonorossi commented Jul 7, 2014

ruba1987 commented Jul 7, 2014

ruba1987 commented Sep 9, 2014

jonorossi commented Sep 10, 2014

jonorossi commented Sep 10, 2014

kkozmic commented Sep 10, 2014

jonorossi commented Sep 10, 2014

ruba1987 commented Sep 10, 2014

[Feature] Serilog w/ NLog support {WIP} #61

[Feature] Serilog w/ NLog support {WIP} #61

Conversation

ruba1987 commented Jun 20, 2014

jonnii commented Jun 21, 2014

jonorossi Jun 23, 2014

Choose a reason for hiding this comment

ruba1987 commented Jun 30, 2014

jonnii Jul 1, 2014

Choose a reason for hiding this comment

jonorossi commented Jul 1, 2014

ruba1987 commented Jul 1, 2014

jonorossi commented Jul 2, 2014

ruba1987 Jul 2, 2014

Choose a reason for hiding this comment

jonorossi Jul 2, 2014

Choose a reason for hiding this comment

ruba1987 Jul 2, 2014

Choose a reason for hiding this comment

ruba1987 commented Jul 2, 2014

jonorossi commented Jul 2, 2014

jonorossi commented Jul 2, 2014

ruba1987 commented Jul 2, 2014

jonorossi commented Jul 2, 2014

ruba1987 commented Jul 2, 2014

ruba1987 commented Jul 2, 2014

jonorossi Jul 3, 2014

Choose a reason for hiding this comment

ruba1987 commented Jul 3, 2014

jonorossi commented Jul 4, 2014

ruba1987 commented Jul 4, 2014

ruba1987 commented Jul 6, 2014

jonorossi commented Jul 7, 2014

ruba1987 commented Jul 7, 2014

ruba1987 commented Sep 9, 2014

jonorossi commented Sep 10, 2014

jonorossi commented Sep 10, 2014

kkozmic commented Sep 10, 2014

jonorossi commented Sep 10, 2014

ruba1987 commented Sep 10, 2014