-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Implement startAfter
for RTDB queries
#7209
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
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 still need to do a thorough pass on the tests, but I did look through all other source files. Looks pretty good!
FirebaseDatabase/Sources/Public/FirebaseDatabase/FIRDatabaseQuery.h
Outdated
Show resolved
Hide resolved
FirebaseDatabase/Sources/Public/FirebaseDatabase/FIRDatabaseQuery.h
Outdated
Show resolved
Hide resolved
FirebaseDatabase/Sources/Public/FirebaseDatabase/FIRDatabaseQuery.h
Outdated
Show resolved
Hide resolved
@schmidt-sebastian Ready for another round of review. |
FirebaseDatabase/Sources/Public/FirebaseDatabase/FIRDatabaseReference.h
Outdated
Show resolved
Hide resolved
unsigned long i = [next length] - 1; | ||
|
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.
This suggestion removes the empty line between the related statements in line 82 and 84.
@schmidt-sebastian Again, thoughts on merge order here? |
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.
You can merge as you please :)
FirebaseDatabase/CHANGELOG.md
Outdated
@@ -1,4 +1,5 @@ | |||
# v7.5.0 | |||
- [added] Implmemented `queryStartingAfterValue` for FirebaseDatabase queries. |
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.
As mentioned in the other PR, it might be worth mentioning the pagination use case here.
if ([next characterAtIndex:i] != [MAX_PUSH_CHAR characterAtIndex:0]) { | ||
break; | ||
} | ||
i--; |
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--; | |
--i; |
From the Google style guide: "Use the prefix form (++i) of the increment and decrement operators unless you need postfix semantics."
withString:MAX_PUSH_CHAR | ||
startingAtIndex:0]; | ||
NSString *maxKeySuccessor = [FNextPushId successor:maxKey]; | ||
XCTAssertEqualObjects(maxKeySuccessor, [FUtilities maxName], @""); |
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.
You can skip the last argument (or provide a description).
* Implement `endBefore` for RTDB queries * Write tests, various other cleanup * fixup predecessor function * CHANGELOG.md * typo * 2021+ copyrights need LLC * cleanup * style * Update FirebaseDatabase/Sources/Utilities/FNextPushId.m Co-authored-by: Sebastian Schmidt <[email protected]> Co-authored-by: Sebastian Schmidt <[email protected]>
startAfter
for RTDB queriesstartAfter
and endBefore
for RTDB queries
startAfter
and endBefore
for RTDB queriesstartAfter
for RTDB queries
API Changes
go/rtdb-exclusive-query-filters