Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Log when deleteAll is called with an alarm still set on the object #2630

Merged
merged 1 commit into from
Aug 30, 2024

Conversation

a-robinson
Copy link
Member

To get a better sense of the prevalence of this usage pattern.

@kentonv
Copy link
Member

kentonv commented Aug 30, 2024

Should this use LOG_PERIODICALLY in case this is very common?

@a-robinson a-robinson force-pushed the arobinson/log-deleteall-alarm branch 2 times, most recently from 5a83db6 to 597d20b Compare August 30, 2024 16:22
@a-robinson
Copy link
Member Author

Should this use LOG_PERIODICALLY in case this is very common?

I was hoping to avoid that so that we could get more complete data, operating under the assumption that deleteAll itself is only used very infrequently, and that it should be even less frequent in combination with alarms. But now I feel obligated to since it'll look silly if that assumption ends up being wrong 🙃

Switched it over.

@a-robinson a-robinson force-pushed the arobinson/log-deleteall-alarm branch from 597d20b to e85eee8 Compare August 30, 2024 17:09
@a-robinson a-robinson requested a review from a team as a code owner August 30, 2024 17:09
To get a better sense of the prevalence of this usage pattern.
@a-robinson a-robinson force-pushed the arobinson/log-deleteall-alarm branch from e85eee8 to 5ffbf6b Compare August 30, 2024 17:11
@a-robinson a-robinson merged commit 2f13452 into main Aug 30, 2024
13 checks passed
@a-robinson a-robinson deleted the arobinson/log-deleteall-alarm branch August 30, 2024 18:36
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.

4 participants