Skip to content

Follow-up to #29901#30003

Merged
tavplubix merged 3 commits intomasterfrom
followup_29901
Oct 13, 2021
Merged

Follow-up to #29901#30003
tavplubix merged 3 commits intomasterfrom
followup_29901

Conversation

@tavplubix
Copy link
Copy Markdown
Member

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

Detailed description / Documentation draft:
Follow-up to #29901, avoids messages like this:
"Finalizing session 1: finalization_started=false, queue_closed=false, reason=unknown"

@robot-clickhouse robot-clickhouse added the pr-not-for-changelog This PR should not be mentioned in the changelog label Oct 11, 2021
{
if (zk)
zk->finalize();
zk->finalize("config changed");
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.

LGTM, but... all messages are from Capital letter but this not. Let's fix it.

Copy link
Copy Markdown
Member Author

@tavplubix tavplubix Oct 11, 2021

Choose a reason for hiding this comment

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

Actually all messages start from lowecase letter (see #29901), but "Operation timeout" starts from a capital because we already have "Operation timeout" in other places and it will be more convenient to grep. Will change other messages to start from a capital too for consistency.

@alexey-milovidov alexey-milovidov self-assigned this Oct 11, 2021
@tavplubix tavplubix merged commit a69b0d9 into master Oct 13, 2021
@tavplubix tavplubix deleted the followup_29901 branch October 13, 2021 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-not-for-changelog This PR should not be mentioned in the changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants