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

Introduce Quality::Unknown variant #3206

Merged
merged 3 commits into from
Jan 3, 2025

Conversation

MartinquaXD
Copy link
Contributor

Description

Sparked by this discussion I think that using Option<Quality> is a bit implicit.

Changes

I adjusted the bad token detection code to use Quality::Unknown where possible to make it more explicit what the strategy thinks about a token.

This makes some of the code more verbose so I'm not super sold on this change yet. Let me know if you think this improves things.

@MartinquaXD MartinquaXD requested a review from a team as a code owner January 3, 2025 11:35
Copy link
Contributor

@sunce86 sunce86 left a comment

Choose a reason for hiding this comment

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

Some parts are better, some worse (with regards to readability), but overall I prefer to merge this.

crates/driver/src/domain/competition/bad_tokens/cache.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@m-lord-renkse m-lord-renkse left a comment

Choose a reason for hiding this comment

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

Thank you! It looks great!

@MartinquaXD MartinquaXD merged commit 50eb0a0 into main Jan 3, 2025
11 checks passed
@MartinquaXD MartinquaXD deleted the make-unknown-state-more-explicit branch January 3, 2025 14:51
@github-actions github-actions bot locked and limited conversation to collaborators Jan 3, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants