-
Notifications
You must be signed in to change notification settings - Fork 1
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
explorer default sort by score #3689
Conversation
64f34b9
to
0adf3dd
Compare
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 like the extra test coverage and I think the results will make more sense.
0adf3dd
to
fae56bc
Compare
}, | ||
want: []string{"someName", "otherName"}, | ||
want: []string{"someName"}, |
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.
Reducing the number of results given that without any ordering, the default is score, and given * there is no indication of quality ... we have both results with score of 1 ... which we cannot depend on its order reliability for asserting ...
given the test is that we could query by * ... having one or more than one might not matter much
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.
Still LGTM. The new test logic makes sense.
…and we cannot deterministically rely on order
ee3e091
to
6dcb162
Compare
Closes #3627
What changed?
Why was this change made?
How did you validate the change?
Tilt on the branch and search for flux-system you should see now flux-system objects ranking first
Integration tests -- what is covered, what cannot be covered;
or, explain why there are no new tests
Unit tests -- what is covered, what cannot be covered; are
there tests that fail without the change?
Added unit test to ensure deterministic search by scoring behaviour
Release notes
Documentation Changes
Other follow ups