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

README maven logger should be runtime, not compile #507

Merged
merged 1 commit into from
Jun 5, 2021

Conversation

paulvi
Copy link
Contributor

@paulvi paulvi commented Mar 18, 2021

should be runtime, not compile dependencies.

Copy link
Collaborator

@philsttr philsttr left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @paulvi !

I agree that the example should use runtime by default, since the most common usage of logstash-logback-encoder is xml-configuration only.

However, logstash-logback-encoder provides many java extension points, for example:

If any of these extension points are used, then compile scope is needed.

Therefore, if the default example in the readme is changed to runtime scope, then a note needs to be added that describes A) when runtime scope can be used safely, and B) when compile scope must be used instead.

@philsttr philsttr merged commit 7d3c937 into logfellow:master Jun 5, 2021
@philsttr philsttr added this to the 7.0 milestone Jun 5, 2021
@paulvi paulvi deleted the patch-1 branch June 7, 2021 10:15
@paulvi
Copy link
Contributor Author

paulvi commented Jun 7, 2021

Thanks,
README now has details

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