-
Notifications
You must be signed in to change notification settings - Fork 166
SearchOrder as part of the API (no impl yet) #364
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
Conversation
|
May I ask what we are going to use this for? We have only one search box. Do we want to let the user choose (via some drop-down menu) how to rank? I would assume we just need to tweak the way we combine the different metrics in order to get good results. |
Immediately: I want to replace Short term: Query the most popular packages (optional: group by platform), to be displayed on the site's index page. After that: It kind of enables a search-box like the top-left side of https://www.npmjs.com/search?q=
I believe that is correct, I don't imagine too much work needed to implement it. We will still need the API part for it though... |
mkustermann
left a comment
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.
Thanks for the explanation, sounds reasonable.
just a few minor comments.
app/lib/shared/search_service.dart
Outdated
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.
Why not just change {SearchOrder orElse()} to {SearchOrder defaultsTo}? Then call sites don't need to allocate a closure but can just provide a constant.
app/lib/shared/search_service.dart
Outdated
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.
defaultsTo: SearchOrder.overall
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.
If you write a test, then you might as well add a case for parseSearchOrder('foobar', defaultsTo: ...) :)
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.
added
app/lib/shared/search_service.dart
Outdated
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.
What's the difference between health and maintenance? For me, not knowing what it means, I would assume it's the same as health.
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.
-
health: How the code qualifies against our tests (e.g. strong mode, some linter rules, works with the tools, doesn't have too many warnings)? E.g. I can have a lot of code in it that is using a deprecated API and it will likely break -> bad health.
-
maintenance: How frequently is the package updated with new versions? E.g. If I publish a new version every 2-3 months, that seems like a steady maintenance. If I publish a new version after 2 years of break, that should score lower, even though it may be fresh. (There is a fundamental difference here with
updated, which only cares about the latest publish time.)
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.
Yeah but you can interpret update frequency in different ways: Frequent updates (might indicate active maintenance, or unstable package) or very rarely (might indicate stable package, or unmaintained package)!
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.
Good point. I'll remove it for now, it should be cheap to re-introduce it later.
No description provided.