Refactor rollup #4184
Refactor rollup #4184
Conversation
![pullrequest[bot]](https://avatars.githubusercontent.com/in/3434?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ A review job has been created and sent to the PullRequest network.
@animesh2049 you can click here to see the review status or cancel the code review job.
posting/list.go
Outdated
| plist = out.plist | ||
| endUid = math.MaxUint64 | ||
| } else { | ||
| init() |
There was a problem hiding this comment.
Reviewed the execution flow and had a question. When init is called on line 835, is it possible to also be called on line 843?
There was a problem hiding this comment.
Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @manishrjain, @martinmr, and @pullrequest[bot])
posting/list.go, line 834 at r1 (raw file):
Previously, pullrequest[bot] wrote…
Reviewed the execution flow and had a question. When
initis called on line 835, is it possible to also be called on line 843?
l.plist.Splits isn't changing, so this check isn't really needed there.
There was a problem hiding this comment.
Looks alright from my end. Defer to @martinmr for final approval.
Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on @animesh2049, @manishrjain, @martinmr, and @pullrequest[bot])
posting/list.go, line 814 at r1 (raw file):
// Method to properly initialize all the variables described above. init := func() {
update
There was a problem hiding this comment.
Just address the comment on the function name.
Reviewed 1 of 1 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @manishrjain and @pullrequest[bot])
posting/list.go, line 814 at r1 (raw file):
Previously, manishrjain (Manish R Jain) wrote…
update
rename to something like initializeSplit.
The purpose of this method is to move us to the next list in a multi-part list when the iteration detects that a list boundary has been crossed.
Also update the documentation for this method to
// Method to properly initialize the variables above when a multi-part list boundary is crossed.
since the current one is pretty generic.
There was a problem hiding this comment.
Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on @manishrjain, @martinmr, and @pullrequest[bot])
posting/list.go, line 814 at r1 (raw file):
Previously, martinmr (Martin Martinez Rivera) wrote…
rename to something like initializeSplit.
The purpose of this method is to move us to the next list in a multi-part list when the iteration detects that a list boundary has been crossed.
Also update the documentation for this method to
// Method to properly initialize the variables above when a multi-part list boundary is crossed.since the current one is pretty generic.
Done.
There was a problem hiding this comment.
Reviewed 1 of 1 files at r2.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @manishrjain and @pullrequest[bot])
Non multi-part list check doesn't need to happen again and again.
This change is