Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added constructors with name argument to the target types #1434

Merged
merged 12 commits into from
May 10, 2016

Conversation

304NotModified
Copy link
Member

@304NotModified 304NotModified commented May 4, 2016

Added "name" to the constructors of all the Targets.

  • For regular targets : added constructor target(string name)
  • For Wrapper targets: added constructor target(string name, params Target target)
  • For Wrapper targets: added constructor target(string name, params Target[] targetst)

supersedes #1370

fixes #1297

Thanks @flyingcroissant !

flyingcroissant and others added 8 commits May 4, 2016 22:15
Resolves issues #1297. Added constructors to the Target classes with a
string parameter for the Target name. Added a unit test to check that
each Target class can be constructed with new constructor and default
constructor and that objects will have the same property values (other
than name).
Changed calls to PropertyInfo.GetValue in unit test to work on other
platforms than .NET 4.5.
Compare all properties, not just public ones. Need to filter SyncRoot
since it will differ per instance. Fix comparison to properly compare
values of properties from each instance.
Execute  efreshFileArchive() + RefreshArchiveFilePatternToWatch() only after first initialize (because of potentional stackoverflow)
@codecov-io
Copy link

codecov-io commented May 4, 2016

Current coverage is 75.63%

Merging #1434 into master will increase coverage by +0.34%

  1. 5 files in src/NLog/Targets were modified. more
    • Misses -19
    • Partials +3
    • Hits +16
  2. File ...og/Targets/Target.cs (not in diff) was modified. more
    • Misses 0
    • Partials -1
    • Hits +1
@@             master      #1434   diff @@
==========================================
  Files           268        268          
  Lines         16049      16213   +164   
  Methods           0          0          
  Messages          0          0          
  Branches       1737       1738     +1   
==========================================
+ Hits          12081      12262   +181   
+ Misses         3546       3526    -20   
- Partials        422        425     +3   

Powered by Codecov. Last updated by ac8190a...bdb8180

@@ -87,13 +87,23 @@ public class AsyncTargetWrapper : WrapperTargetBase
/// Initializes a new instance of the <see cref="AsyncTargetWrapper" /> class.
/// </summary>
public AsyncTargetWrapper()
: this(null)
: this((Target)null)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Breaking?

//clear cache
cachedCleanedFileNamed = null;
}
var simpleLayout = value as SimpleLayout;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix indentation

@304NotModified 304NotModified modified the milestone: 4.3.4 May 10, 2016
@bhaeussermann
Copy link
Contributor

AppVeyor/pr passed, so I assume it is safe to merge this.

@bhaeussermann bhaeussermann merged commit 3baae2f into master May 10, 2016
@304NotModified 304NotModified deleted the pr-1370_2 branch May 10, 2016 18:34
@304NotModified
Copy link
Member Author

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants