-
Notifications
You must be signed in to change notification settings - Fork 945
Port IndexState and IndexOffset #5916
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
Changeset File Check ✅
|
Size Report 1Affected Products
Test Logs |
Size Analysis Report 1Affected Products
Test Logs |
bc196ab
to
993228f
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.
lgtm
return primitiveComparator(left.kind, right.kind); | ||
} | ||
|
||
/** Stores the "high water mark" that indicates how updated the Index is for the current user. */ |
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.
ubernit: line wrap
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.
Fixed
(options.fields ?? []).map( | ||
entry => new IndexSegment(field(entry[0]), entry[1]) | ||
), | ||
new IndexState(-1, options.offset ?? IndexOffset.min()) |
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.
nit: pref using global const for -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.
Done
} | ||
|
||
for ( | ||
let i = 0; |
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.
optional: if the left and right segments don't match in length, we can exit early rather than comparing, but i can also see how this code is less verbose
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.
That works for equals, but not here since "b,c" sorts after "a".
} | ||
|
||
/** Creates a new offset based on the provided document. */ | ||
export function newIndexOffsetFromDocument(document: Document): IndexOffset { |
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 double checking that you plan on adding another method to create a new index offset with a specified largest batch id.
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.
The equivalent for Web is just calling the constructor of IndexOffset.
No description provided.