Skip to content

Conversation

@elbeno
Copy link
Contributor

@elbeno elbeno commented Nov 3, 2025

Problem:

  • The flow services and builders are too coupled to their implementations, viz:
    • flow::FunctionPtr is used for both the interface function on the service and the node type for the inlined function list; these have nothing to do with each other.
    • The service needs to know about the builder's interface.
    • The graph_builder knows about details of inlining the function list.
    • The impl is poorly named (it was named for the first, but now not the only, implementation).

Solution:

  • Inject the builders and implementations at the correct levels.
  • Instead of passing through template arguments, expose names and interfaces from lower levels. This enables concepts (TBD).

Problem:
- The flow `service`s and `builder`s are too coupled to their implementations, viz:
  - `flow::FunctionPtr` is used for both the interface function on the `service`
    and the node type for the inlined function list; these have nothing to do
    with each other.
  - The `service` needs to know about the builder's interface.
  - The `graph_builder` knows about details of inlining the function list.
  - The `impl` is poorly named (it was named for the first, but now not the
    only, implementation).

Solution:
- Inject the builders and implementations at the correct levels.
- Instead of passing through template arguments, expose names and interfaces
  from lower levels. This enables concepts (TBD).
Copy link
Contributor

@mjcaisse-intel mjcaisse-intel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(praise) Really nice rework.

@elbeno elbeno merged commit 4e15633 into intel:main Nov 4, 2025
33 checks passed
@elbeno elbeno deleted the clean-up-flow branch November 4, 2025 22:21
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.

2 participants