Skip to content

Add FlushStrategyResult. Add strategy_info to flush history.#36943

Merged
arnej27959 merged 3 commits into
masterfrom
toregge/add-flush-strategy-result
May 21, 2026
Merged

Add FlushStrategyResult. Add strategy_info to flush history.#36943
arnej27959 merged 3 commits into
masterfrom
toregge/add-flush-strategy-result

Conversation

@toregge
Copy link
Copy Markdown
Member

@toregge toregge commented May 19, 2026

@vekterli : please review
@arnej27959 : FYI

@toregge toregge requested a review from vekterli May 19, 2026 11:57
vekterli
vekterli previously approved these changes May 20, 2026
Copy link
Copy Markdown
Member

@vekterli vekterli left a comment

Choose a reason for hiding this comment

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

👍

}

const std::string strategy_name("memory");
const std::string no_info("none");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Philosophical question: is it best to communicate no info with an explicit "none" string or an empty string? 🤔

if (LOG_WOULD_LOG(event)) {
EventLogger::flushInit(it->getName());
auto& strategy_info = flush_strategy_result.strategy_info();
auto& strategy_name = flush_strategy_result.strategy_name();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Consider making auto-refs const auto& to make it more immediately obvious that they are non-mutable refs

Copy link
Copy Markdown
Member

@vekterli vekterli left a comment

Choose a reason for hiding this comment

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

👍

@arnej27959 arnej27959 merged commit b5b489f into master May 21, 2026
3 checks passed
@arnej27959 arnej27959 deleted the toregge/add-flush-strategy-result branch May 21, 2026 07: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