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

feat: add flags for toggling inclusivity on fetchByScore scores #238

Merged
merged 1 commit into from
Oct 21, 2024

Conversation

nand4011
Copy link
Contributor

Add $inclusiveMin and $inclusiveMax to sortedSetFetchByScore to allow callers to toggle whether the min and max scores should be inclusive or exclusive. They default to inclusive.

Add tests for the flags.

Change the behavior for an unrecognized sort order from defaulting to ascending to returning an error.

Make $order not nullable. There isn't a reason for it to be null and it has a default argument.

Add $inclusiveMin and $inclusiveMax to sortedSetFetchByScore to allow
callers to toggle whether the min and max scores should be inclusive
or exclusive. They default to inclusive.

Add tests for the flags.

Change the behavior for an unrecognized sort order from defaulting to
ascending to returning an error.

Make $order not nullable. There isn't a reason for it to be null and it
has a default argument.
Copy link
Contributor

@rishtigupta rishtigupta left a comment

Choose a reason for hiding this comment

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

:shipit:

if (!function_exists('validateSortedSetOrder')) {
function validateSortedSetOrder(int $order): void
{
if ($order != SORT_ASC && $order != SORT_DESC) {
Copy link
Contributor

Choose a reason for hiding this comment

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

💪

@nand4011 nand4011 merged commit a8cfed8 into main Oct 21, 2024
5 checks passed
@nand4011 nand4011 deleted the fetch-by-score-inclusive-flag branch October 21, 2024 21:52
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