Skip to content

Conversation

@lizhanhui
Copy link
Contributor

…j2 related libraries as test

@lizhanhui lizhanhui changed the title Fix https://github.com/openmessaging/dledger/issues/195, scoping log4… [Issue 195] Scoping log4j libraries to test Aug 3, 2022
@lizhanhui lizhanhui changed the title [Issue 195] Scoping log4j libraries to test [Issue #195] Scoping log4j libraries to test Aug 3, 2022
@lizhanhui lizhanhui merged commit b7e559d into openmessaging:master Aug 8, 2022
@lizhanhui lizhanhui deleted the develop branch August 8, 2022 03:50
@RongtongJin
Copy link
Contributor

@lizhanhui Although DLedger is a library, it is also a program that can run independently. If all log implementation is set to test scope, this will result in no log output even for quick start. In addition, DO NOT MERGE your own PR without anyone's approval.

@lizhanhui
Copy link
Contributor Author

lizhanhui commented Aug 15, 2022

Although DLedger is a library, it is also a program that can run independently.

Then you should split your library and program, and configure the runnable program with appropriate logging infra.

If all log implementation is set to test scope, this will result in no log output even for quick start.

Well engineered and designed project won't have this issue at all.

In addition, DO NOT MERGE your own PR without anyone's approval.

The issue and pull request stayed unprocessed for more than 72 hours. And it was added as a required dependency for the core project. And it now turns into a blocker issue. Make it optional or make it well engineered.

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