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

[GRIFFIN-273] Griffin Service with user-defined log4j2 configuration #518

Closed
wants to merge 13 commits into from

Conversation

joohnnie
Copy link
Contributor

Currently, the Griffin service module use spring-boot default log4j2 configuration.

This approach is not easy to change the config for debugging and add new appender such as
RollingFile

@guoyuepeng
Copy link
Contributor

hi Joohnnie,
Could you rebase, some commits are already merged before.

@joohnnie
Copy link
Contributor Author

Hi William,
I have rebased and please help to check.

Thanks
Johnnie

@guoyuepeng
Copy link
Contributor

Thanks, Johnnie.

@whhe
Copy link
Member

whhe commented Aug 5, 2019

It seems that your configuration is based on the default configuration of spring boot, I think there is no problem with the content.

But I'm not sure if giving an implementation like what you do or giving a guide in a document is better, cause If users want customized log, they may still need to make some changes in these files.

@joohnnie
Copy link
Contributor Author

joohnnie commented Aug 5, 2019

@whhe Thanks for your comments.
The config is the default configuration from spring boot. The reason I put it to the code base because we can easily change the config based on our need. Otherwise, users even don't know what' the log library they used.
In addition to that, I removed some dependencies as they will cause the confliction in the log system.

Regarding the guild, log4j2 is an open-source library and has its official website.
User can refer to the website to customize the config.
https://logging.apache.org/log4j/2.x/index.html

@@ -82,6 +82,10 @@ under the License.
</exclusion>
</exclusions>
</dependency>
<dependency>
Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM.

@guoyuepeng
Copy link
Contributor

LGTM.

@asfgit asfgit closed this in 2945fd2 Aug 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants