You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I've added this issue to track a discussion related to adding Edge support to the DSL. I'm happy to push up a PR, but I'd like to make sure it's in the style you would like to see. A few questions:
Adding edge support will require a new Converter.forDomainEdge implicit. I believe this will require me to create a new DomainEdge marker trait that extends DomainRoot, for use in the Converter.forDomainEdge context bound:
I'm wondering if we should also add a DomainNode marker trait that extends DomainRoot, so that the context-bound of Converter.forDomainNode can be more specific. That would be a breaking change, which is why I bring it up. It seems like the right thing to do now, though.
It seems like a lot of the things that are going in to NodeSteps that could be shared between NodeSteps and EdgeSteps, so I propose creating an ElementSteps base class and pushing much of what's in NodeSteps into ElementSteps. It probably still makes sense to leave NodeSteps there, even if it is empty, simply to fix the EndGraph type-parameter to Vertex, and give ourselves a future location for steps that are truly vertex-specific.
I think some of the things in NodeSteps should really be in Steps, such as filterOnEnd and sideEffect, which could apply to any EndDomain (such as a selected tuple).
Some of the new steps you've added to NodeSteps aren't using the implicit Constructor pattern that you were using earlier. I presume you're expecting that there will be an implicit to lift NodeSteps to its domain-specific subclass. Is there a reason why you're moving away from the Constructor pattern? Should we rely on the runtime conversion moving forward, or should we prefer the Constructor pattern?
The text was updated successfully, but these errors were encountered:
As you've figured, I only handle vertices so far, simply because that's all we need at the moment (at my work). Introducing breaking changes for keeping the code clean is not a problem.
1/2/3) yes that all makes sense
the constructor pattern is adding quite some complexity and is only used where absolutely needed, i.e. Steps.map and Steps.flatMap, but nowhere in NodeSteps. Maybe we can even remove it there - I'd actually appreciate that, but am open to other suggestions.
Thanks Jeremy!
I've added this issue to track a discussion related to adding Edge support to the DSL. I'm happy to push up a PR, but I'd like to make sure it's in the style you would like to see. A few questions:
I'm wondering if we should also add a DomainNode marker trait that extends DomainRoot, so that the context-bound of Converter.forDomainNode can be more specific. That would be a breaking change, which is why I bring it up. It seems like the right thing to do now, though.
It seems like a lot of the things that are going in to NodeSteps that could be shared between NodeSteps and EdgeSteps, so I propose creating an ElementSteps base class and pushing much of what's in NodeSteps into ElementSteps. It probably still makes sense to leave NodeSteps there, even if it is empty, simply to fix the EndGraph type-parameter to Vertex, and give ourselves a future location for steps that are truly vertex-specific.
I think some of the things in NodeSteps should really be in Steps, such as filterOnEnd and sideEffect, which could apply to any EndDomain (such as a selected tuple).
Some of the new steps you've added to NodeSteps aren't using the implicit Constructor pattern that you were using earlier. I presume you're expecting that there will be an implicit to lift NodeSteps to its domain-specific subclass. Is there a reason why you're moving away from the Constructor pattern? Should we rely on the runtime conversion moving forward, or should we prefer the Constructor pattern?
The text was updated successfully, but these errors were encountered: