Skip to content

vyzo's review of the dagstore#93

Draft
vyzo wants to merge 1 commit intomasterfrom
vyzo/review
Draft

vyzo's review of the dagstore#93
vyzo wants to merge 1 commit intomasterfrom
vyzo/review

Conversation

@vyzo
Copy link

@vyzo vyzo commented Aug 2, 2021

Haven't dived into the subdirectories yet.

@vyzo vyzo requested a review from raulk August 2, 2021 11:54
@vyzo
Copy link
Author

vyzo commented Aug 2, 2021

So far one minor bug (see comments inline) and one recommendation for refactoring:
The task processing logic of the event loop should be extracted on its own method, which can then:

  • lock/defer unlock to avoid having a really long lock scope with a bunch of pasta in-between
  • return the next task and avoid the internaclCh indirection, which can now be removed altogether.

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.

1 participant