add final param to AddRule Methods

[893949088]

add final param by AddRule Method

[snakefoot]

This is a breaking change. You have to make new methods, instead of adding parameter to existing ones.

[893949088]

@snakefoot I don‘t want to configure nlog by xml file.I want configure nlog by LoggingConfiguration.but the AddRule method don't has 'final' param. why this is a breaking change?

[snakefoot]

why this is a breaking change?

Your code-change requires a complete recompile of all source-code using NLog, or else they will get a MissingMethodException:

Ex. This method is suddenly gone:

public void AddRuleForOneLevel(LogLevel level, Target target, string loggerNamePattern)

Instead there is this method, but it has a different signature:

public void AddRuleForOneLevel(LogLevel level, Target target, string loggerNamePattern, bool final = false)

[893949088]

@snakefoot So, Should I open a new issue??

[snakefoot]

@893949088 You can just add more commits to the same PR. They will usually be squashed into a single commit before merge.

[893949088]

@snakefoot But I don't have more change to commit. I really want this ’final‘ function。
╯﹏╰ !!

[snakefoot]

@893949088 If you want this included within the next year, then you need to fix the breaking change. Else it will be postponed until like forever.

[893949088]

@snakefoot I'm very sad. why is the 'LoggingConfiguration' imperfect？

[snakefoot]

why is the 'LoggingConfiguration' imperfect？

Because of the breaking change you have introduced, by changing the signature of the existing methods.

[893949088]

@snakefoot How can I do it to fix the breaking change？I haven't submit PR before.
^_^"

[snakefoot]

@893949088 The breaking change is that you have modified the signature of the existing methods. The very simple solution is not to modify the signature of the existing methods.

So just add the new methods without default parameters, and make the existing methods call the new method:

public void AddRuleForOneLevel(LogLevel level, Target target, string loggerNamePattern = "*")
{
   // Existing method forwarding to new method
   AddRuleForOneLevel(level, target, loggerNamePattern, false);
}

public void AddRuleForOneLevel(LogLevel level, Target target, string loggerNamePattern, bool final)
{
   // Do the right thing
}

[893949088]

@snakefoot Thank you very much!!!

[codecov]

Codecov Report

Merging #2612 into master will increase coverage by <1%.
The diff coverage is 83%.

@@           Coverage Diff           @@
##           master   #2612    +/-   ##
=======================================
+ Coverage      81%     81%   +<1%     
=======================================
  Files         325     325            
  Lines       23938   23950    +12     
  Branches     3025    3028     +3     
=======================================
+ Hits        19416   19427    +11     
- Misses       3704    3708     +4     
+ Partials      818     815     -3

[893949088]

〒_〒！〒_〒！〒_〒！

[snakefoot]

@893949088 Remember to remove the parameter default values for the new methods.

[304NotModified]

looks good, thanks!

Will merge this when Travis gives green (restarted Travis, it was stuck)