-
Notifications
You must be signed in to change notification settings - Fork 230
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
Bug 1904132 - Suggest CLI #6280
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6280 +/- ##
=======================================
Coverage ? 22.64%
=======================================
Files ? 330
Lines ? 29814
Branches ? 0
=======================================
Hits ? 6750
Misses ? 23064
Partials ? 0 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one little suggestion. Also, since it's just local, it's probably fine to have .unwrap()
s. I personally would log
them via eprintln!
at least, so in case something goes wrong, it's easier to see where the problem is.
examples/suggest-cli/src/main.rs
Outdated
} | ||
|
||
fn query(store: &SuggestStore, provider: SuggestionProviderArg, input: String) { | ||
let provider = match provider { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know it's "just" for local testing, but would be a bit cleaner to have a impl From>SuggestionProviderArg>
so we can do let provider = SuggestionProvider::from(provider_args);
impl From<SuggestionProviderArg> for SuggestionProvider {
fn from(arg: SuggestionProviderArg) -> Self {
match arg {
SuggestionProviderArg::Amp => SuggestionProvider::Amp,
SuggestionProviderArg::Wikipedia => SuggestionProvider::Wikipedia,
SuggestionProviderArg::Amo => SuggestionProvider::Amo,
SuggestionProviderArg::Pocket => SuggestionProvider::Pocket,
SuggestionProviderArg::Yelp => SuggestionProvider::Yelp,
SuggestionProviderArg::Mdn => SuggestionProvider::Mdn,
SuggestionProviderArg::Weather => SuggestionProvider::Weather,
SuggestionProviderArg::AmpMobile => SuggestionProvider::AmpMobile,
SuggestionProviderArg::Fakespot => SuggestionProvider::Fakespot,
}
}
}
Added the `suggest-cli` crate that implements a simple clap CLI to ingest and query suggestions. This can be run using `cargo suggest`. My plan is to use this to test fakespot blocklist support.
Thanks for the suggestions, the code is looking much better now. I ended up replacing the unwraps with unwrap_or_else plus a panic. That should lead to an okay error message being printed out. |
Added the
suggest-cli
crate that implements a simple clap CLI to ingest and query suggestions. This can be run usingcargo suggest
.My plan is to use this to test fakespot blocklist support.
Pull Request checklist
[ci full]
to the PR title.Branch builds: add
[firefox-android: branch-name]
to the PR title.