Skip to content

Extract structs that encode announcements to users #194

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

Merged
merged 1 commit into from
May 26, 2025

Conversation

blairconrad
Copy link
Contributor

Part of #192.

Does not aid testability at this point, but provides a consistent mechanism for creating "announcements" that are conveyed to the user during normal (or even abnormal operation). Helps ensure these are properly crafted and distinguishes them from debug-logs.

Copy link
Owner

@tummychow tummychow left a comment

Choose a reason for hiding this comment

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

ah, so you chose to roll it yourself, not bad

src/lib.rs Outdated
/// normal operations (not debug messages).
trait Announceable {
fn announce_to(&self, logger: &slog::Logger);
}
Copy link
Owner

Choose a reason for hiding this comment

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

rather than having a trait implemented by each announcement, it would be tidier to have an enum comprising all possible announcements...

src/lib.rs Outdated
}

fn announce<T: Announceable>(logger: &slog::Logger, announcement: T) {
announcement.announce_to(logger);
Copy link
Owner

Choose a reason for hiding this comment

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

... and then instead of calling a method, announce simply matches on the enum. the match will be long ofc, but not that different to the current arrangement of code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah. rust has "rich enums" of a sort. Learning things every day.
I think I did the thing you asked.
I just amended because there's a lot of churn between the two commits; no benefit in seeing the wandering path, IMO.

@blairconrad
Copy link
Contributor Author

ah, so you chose to roll it yourself, not bad

Sometimes "didn't know any better options" can look like "chose". The only other thing I was aware of that would do something close to this was the slog::Value, but it looked to me like this is great for logging parts of the msg, but not the key-value pairs.

@tummychow tummychow merged commit 1a24dba into tummychow:master May 26, 2025
6 checks passed
@blairconrad blairconrad deleted the announce-structs branch May 26, 2025 20:31
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.

2 participants