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

Fix typos in logback.xml configuration files. #308

Merged
merged 3 commits into from
Aug 8, 2017
Merged

Fix typos in logback.xml configuration files. #308

merged 3 commits into from
Aug 8, 2017

Conversation

priornix
Copy link
Contributor

@priornix priornix commented Aug 8, 2017

Correct syntax for logback loggers should be <appender-ref> elements instead of <AppenderRef>, which does not work. This can be checked with <configuration debug="true">

https://logback.qos.ch/manual/configuration.html

@priornix
Copy link
Contributor Author

priornix commented Aug 8, 2017

Originally, 8c2eabd fixed the appenders to the correct syntax. However, this will result in duplicate messages, since appenders are additive in nature.

The original config file used <AppenderRef> which appears to work, because <AppenderRef> is an invalid element and logback basically ignored it, thus there were no duplicate log messages.

To avoid confusion, <AppenderRef> should be removed, as
i) it is invalid syntax
ii) it can mislead developers to think that <AppenderRef> is also valid syntax, which I had thought initially, until I looked into why my other logger's appender was not working and discovered the truth from logback's documentation.

@priornix
Copy link
Contributor Author

priornix commented Aug 8, 2017

Last commit 757ec65 changes <logger ...></logger> to <logger ... /> to reduce verbosity since there are no child elements. Also, neatly align the output so that they are no extra newlines.

@yogthos
Copy link
Member

yogthos commented Aug 8, 2017

Perfect, thanks for getting this cleaned up.

@yogthos yogthos merged commit 34bdf45 into luminus-framework:master Aug 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants