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

Various XSD improvements (NLog.Schema package) #3409

Merged
merged 3 commits into from May 24, 2019

Conversation

304NotModified
Copy link
Member

@304NotModified 304NotModified commented May 20, 2019

fixes #3377

  • fix LoggerName

related #2394 and #2891

@304NotModified 304NotModified changed the title Various XSD improvements Various XSD improvements (NLog.Schema) May 20, 2019
@304NotModified 304NotModified changed the title Various XSD improvements (NLog.Schema) Various XSD improvements (NLog.Schema package) May 20, 2019
@snakefoot
Copy link
Contributor

@304NotModified Think you should also fix this method to recognize RULENAME:

        private LoggingRule ParseRuleElement(ILoggingConfigurationElement loggerElement)
        {
            IEnumerable<LogLevel> enableLevels = null;
            int minLevel = 0;
            int maxLevel = LogLevel.MaxLevel.Ordinal;

            string ruleName = null;
            string namePattern = null;
            bool enabled = true;
            bool final = false;
            string writeTargets = null;
            foreach (var childProperty in loggerElement.Values)
            {
                switch (childProperty.Key?.Trim().ToUpperInvariant())
                {
                    case "NAME":
                        if (loggerElement.MatchesName("logger"))
                            namePattern = childProperty.Value; // Legacy Style
                        else
                            ruleName = childProperty.Value;
                        break;
                    case "RULENAME":
                        ruleName = childProperty.Value; // Legacy Style
                        break;

@304NotModified
Copy link
Member Author

304NotModified commented May 20, 2019

@304NotModified Think you should also fix this method to recognize RULENAME:

This isn't an issue in the XML now isn't?

@304NotModified
Copy link
Member Author

I think we have to update the XSD for NLog 5 and recommend the new approach IMO

@snakefoot
Copy link
Contributor

This isn't an issue in the XML now isn't?

Nope but it would make sense that rulename is recognized by the parser, when adding it to the xml-doc :)

@304NotModified
Copy link
Member Author

ow this is a mistake then. I'm confused why the names aren't in sync...

@304NotModified 304NotModified changed the title Various XSD improvements (NLog.Schema package) Various XSD improvements (NLog.Schema package) (WIP) May 20, 2019
@codecov-io
Copy link

codecov-io commented May 20, 2019

Codecov Report

Merging #3409 into release/4.6.4 will increase coverage by <1%.
The diff coverage is n/a.

@@              Coverage Diff               @@
##           release/4.6.4   #3409    +/-   ##
==============================================
+ Coverage             80%     80%   +<1%     
==============================================
  Files                358     358            
  Lines              28373   28378     +5     
  Branches            3785    3786     +1     
==============================================
+ Hits               22750   22763    +13     
+ Misses              4537    4521    -16     
- Partials            1086    1094     +8

@snakefoot
Copy link
Contributor

Created #3411

@304NotModified 304NotModified changed the title Various XSD improvements (NLog.Schema package) (WIP) Various XSD improvements (NLog.Schema package) May 23, 2019
@304NotModified 304NotModified added this to the 4.6.4 milestone May 23, 2019
@304NotModified 304NotModified merged commit 1cebc99 into release/4.6.4 May 24, 2019
@repo-ranger repo-ranger bot deleted the xsd-add-rulename branch May 24, 2019 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants