-
Notifications
You must be signed in to change notification settings - Fork 47
[refactor]: introduce a 'HostContext' for exposing host info through tree #325
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
@@ -194,7 +202,7 @@ extension WorkflowNode.SubtreeManager { | |||
let eventPipe = EventPipe() | |||
eventPipes.append(eventPipe) | |||
|
|||
/// See if we can | |||
/// See if we can reuse an existing child node for the given key. |
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 truncated sentence has bothered me for a long time – i think this is probably what it was going to
|
||
/// A context object to expose certain root-level information to each node | ||
/// in the Workflow tree. | ||
final class HostContext { |
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.
class?
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.
sorry, are you asking why is it a class? assuming 'yes', then it's because i think we will eventually want to allow different parts of the tree to be able to mutate some of its properties and have those changes be visible elsewhere. e.g. you're handling an action and want to remember that 'some node in the tree was invalidated' by the time the root/host has to do the next render.
0703d92
to
1355990
Compare
HostContext
type to hold 'host-level' information to be distributed to various entities throughout the workflow treenote: no new tests added as no new functionality introduced yet