Skip to content

Conversation

@M4rFri
Copy link

@M4rFri M4rFri commented Aug 15, 2025

Added the option to add a custom log msg to the beginning of each file for the rotating_file_sink

Could be used to add the version of the application in each of the log files..

Copy link

@felixjulianheitmann felixjulianheitmann left a comment

Choose a reason for hiding this comment

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

While I think this feature makes sense and I see no issues with the implementation, I think git-wise this is a little difficult. The v2.x-Attributes-cmake-fixes branch is the one we're trying to merge upstream.

It might make sense to create a "tessonics-main" branch which rebases the changes of v2.x-Attributes-cmake-fixes on top of the v2 main that we then use for us internally. We can then continue implementing on top of that. If at some point the attribute branch would actually get merged this would be solved with a simple rebase.

Additionally regarding the actual code, I feel it's more modular if you add a file_rotation_event_handler function that gets called on rotation instead of providing a log message for this case. Then this can be used for a lot of things and not only for this specific log message. Or the log message can contain the current timestamp of the rotation or anything like that.

@M4rFri
Copy link
Author

M4rFri commented Aug 18, 2025

v2.x-Attributes-cmake-fixes Is already the internally use branch
v2.x-Attributes is the one we try to get merged

I switched to an callback approach

}

TEST_CASE("rotating_file_logger6", "[rotating_logger]") {
auto empty_callback = [](const spdlog::filename_t &) -> std::optional<spdlog::details::log_msg> {

Choose a reason for hiding this comment

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

empty_callback -> copy paste typo

@M4rFri M4rFri merged commit 841a9ea into v2.x-Attributes-cmake-fixes Aug 19, 2025
14 of 24 checks passed
@M4rFri M4rFri deleted the rotating-log-message branch August 19, 2025 06:39
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.

3 participants