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

BugFix: <targets> after <rules> won't work #2113

Merged
merged 2 commits into from
May 22, 2017
Merged

Conversation

304NotModified
Copy link
Member

Fixes #2045

@304NotModified
Copy link
Member Author

supersedes #2096

@304NotModified 304NotModified changed the title Fix: <targets> should be above <rules> BugFix: <targets> should be above <rules> May 16, 2017
@304NotModified 304NotModified changed the title BugFix: <targets> should be above <rules> BugFix: <targets> after <rules> won't work May 16, 2017
@304NotModified 304NotModified modified the milestone: 4.4.10 May 16, 2017
@snakefoot
Copy link
Contributor

Maybe change the logic, so it always parses elements in this order (and not just targets first):

case "EXTENSIONS":
case "INCLUDE":
case "TIME":
case "VARIABLE":
case "APPENDERS":
case "TARGETS":
case "RULES":

@304NotModified
Copy link
Member Author

Indeed this was too Much quick fix

@snakefoot
Copy link
Contributor

Easy fix would be to always parse "RULES" last :)

@304NotModified
Copy link
Member Author

304NotModified commented May 16, 2017

Have to look into this. The location of variables and includes, does matter.

@304NotModified
Copy link
Member Author

Easy fix would be to always parse "RULES" last :)

That's indeed a much easier approach - keep it simple :)

changed!

@codecov
Copy link

codecov bot commented May 21, 2017

Codecov Report

Merging #2113 into master will increase coverage by <1%.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master   #2113    +/-   ##
=======================================
+ Coverage      81%     82%   +<1%     
=======================================
  Files         291     291            
  Lines       20132   20375   +243     
  Branches     2390    2463    +73     
=======================================
+ Hits        16378   16617   +239     
- Misses       3143    3145     +2     
- Partials      611     613     +2

@304NotModified
Copy link
Member Author

Fixed in 4.4.10 and the release is queued. Will be online in a few hours!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug report / Bug fix nlog-configuration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants