deps: Resolve bidirectional logger_system dependency risk#337
Merged
Conversation
Add deprecated option BUILD_WITH_LOGGER_SYSTEM with deprecation message directing users to use common_system ILogger interface instead. This is preparation for removing direct logger_system dependency. Relates to: #336
Mark logger_system_adapter class as deprecated with clear migration guidance pointing to common_system ILogger interface. Relates to: #336
Add functions to register ILogger implementations via ServiceContainer: - register_logger_instance: register existing ILogger - register_logger_factory: register logger factory function - is_logger_registered: check if ILogger is registered - unregister_logger: remove ILogger from container This provides the preferred way to integrate logging with thread_system without direct logger_system dependency. Relates to: #336
Add comprehensive integration tests for ILogger registration with ServiceContainer: - Register logger instance - Register logger factory (singleton/transient) - Resolve and use logger through DI - Log level filtering - Source location capture These tests validate the preferred logging integration path without direct logger_system dependency. Relates to: #336
- Add deprecation and feature entries to CHANGELOG.md - Update LOGGER_INTERFACE_MIGRATION_GUIDE.md with: - logger_system_adapter deprecation notice - DI-based migration examples - Deprecation timeline Relates to: #336
Contributor
📊 Performance Benchmark ResultsPerformance Benchmark ReportNo benchmark data available. ℹ️ No baseline reference availableThis is the first benchmark run or baseline file is missing. |
Add 'namespace common = kcenon::common' before including service_registration.h to resolve ::common:: namespace reference issues in common_executor_adapter.h. Relates to: #336
Contributor
📊 Performance Benchmark ResultsPerformance Benchmark ReportNo benchmark data available. ℹ️ No baseline reference availableThis is the first benchmark run or baseline file is missing. |
The integration test for ILogger DI registration causes namespace conflicts with common_executor_adapter.h's ::common:: references. This will be addressed in a follow-up issue. Relates to: #336
Contributor
📊 Performance Benchmark ResultsPerformance Benchmark ReportNo benchmark data available. ℹ️ No baseline reference availableThis is the first benchmark run or baseline file is missing. |
kcenon
added a commit
that referenced
this pull request
Apr 13, 2026
* cmake: add BUILD_WITH_LOGGER_SYSTEM deprecation warning Add deprecated option BUILD_WITH_LOGGER_SYSTEM with deprecation message directing users to use common_system ILogger interface instead. This is preparation for removing direct logger_system dependency. Relates to: #336 * deprecate: add [[deprecated]] attribute to logger_system_adapter Mark logger_system_adapter class as deprecated with clear migration guidance pointing to common_system ILogger interface. Relates to: #336 * feat(di): add ILogger registration functions Add functions to register ILogger implementations via ServiceContainer: - register_logger_instance: register existing ILogger - register_logger_factory: register logger factory function - is_logger_registered: check if ILogger is registered - unregister_logger: remove ILogger from container This provides the preferred way to integrate logging with thread_system without direct logger_system dependency. Relates to: #336 * test: add ILogger DI integration tests Add comprehensive integration tests for ILogger registration with ServiceContainer: - Register logger instance - Register logger factory (singleton/transient) - Resolve and use logger through DI - Log level filtering - Source location capture These tests validate the preferred logging integration path without direct logger_system dependency. Relates to: #336 * docs: update CHANGELOG and migration guide for Issue #336 - Add deprecation and feature entries to CHANGELOG.md - Update LOGGER_INTERFACE_MIGRATION_GUIDE.md with: - logger_system_adapter deprecation notice - DI-based migration examples - Deprecation timeline Relates to: #336 * fix(test): add namespace alias for common_executor_adapter compatibility Add 'namespace common = kcenon::common' before including service_registration.h to resolve ::common:: namespace reference issues in common_executor_adapter.h. Relates to: #336 * revert: remove integration test due to namespace conflicts The integration test for ILogger DI registration causes namespace conflicts with common_executor_adapter.h's ::common:: references. This will be addressed in a follow-up issue. Relates to: #336
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Resolves the bidirectional dependency risk between thread_system and logger_system by deprecating direct logger_system integration in favor of common_system ILogger interface.
BUILD_WITH_LOGGER_SYSTEMCMake option as deprecated (will be removed in v0.5.0.0)[[deprecated]]attribute tologger_system_adapterclassChanges
CMake
BUILD_WITH_LOGGER_SYSTEMas deprecated option (OFF by default)Code
[[deprecated]]attribute tologger_system_adapterclassservice_registration.h:register_logger_instance(): Register existing ILoggerregister_logger_factory(): Register logger factory (singleton/transient)is_logger_registered(): Check if ILogger is registeredunregister_logger(): Remove ILogger from containerTests
ilogger_di_integration_test.cppwith comprehensive testsDocumentation
CHANGELOG.mdwith deprecation and new feature entriesLOGGER_INTERFACE_MIGRATION_GUIDE.mdwith migration examplesMigration Path
Deprecation Timeline
Test plan
Closes #336