-
Notifications
You must be signed in to change notification settings - Fork 162
Refactor and merge datastore head task pollers. #320
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.
looks fine
(though it's kind of surprising that it adds twice as many lines as it removes)
app/lib/shared/task_sources.dart
Outdated
yield task; | ||
} | ||
} | ||
await reportTurnDone(count); |
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.
Move this statement to the call site.
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.
Do you mean that counting the Stream's length at one level up? To make things less complicated, I think we could operate without the count.
} | ||
await new Future.delayed(const Duration(minutes: 1)); | ||
} | ||
Future<bool> shouldYieldTask(Task task) async { |
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.
Normally I would prefer separating these concerns via composition rather than inheritance, e.g. new FilteredTaskSource(new DatastoreHeadTaskSource())
.
app/lib/search/updater.dart
Outdated
_lastTs = now.subtract(const Duration(minutes: 10)); | ||
await new Future.delayed(new Duration(minutes: 30)); | ||
} | ||
Future reportTurnDone(int count) async { |
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 name does not seem to be very descriptive, maybe dbScanComplete
or something like that.
c895ad3
to
0f8960c
Compare
What would you think about |
Let's just stick with this version for the sake of making progress. It was just a note/recommendation, since composed code is much easier to read/understand than using subclasses. |
The benefit is slim right now, but later on the
dartdoc
service should be able to use the shared one right out of the box.