Graph refactor - #392
Merged
NicolasFussberger merged 26 commits intoAug 5, 2026
Merged
Conversation
WilliamRoebuck
requested a deployment
to
workflow-approval
July 30, 2026 11:49 — with
GitHub Actions
Waiting
WilliamRoebuck
requested a deployment
to
workflow-approval
July 30, 2026 11:49 — with
GitHub Actions
Waiting
License Check Results🚀 The license check job ran with the Bazel command: bazel run --lockfile_mode=error //:license-checkStatus: Click to expand output |
NicolasFussberger
requested a deployment
to
workflow-approval
July 30, 2026 11:53 — with
GitHub Actions
Waiting
NicolasFussberger
requested a deployment
to
workflow-approval
July 30, 2026 11:53 — with
GitHub Actions
Waiting
|
The created documentation from the pull request is available at: docu-html |
WilliamRoebuck
requested a deployment
to
workflow-approval
July 30, 2026 12:13 — with
GitHub Actions
Waiting
WilliamRoebuck
requested a deployment
to
workflow-approval
July 30, 2026 12:13 — with
GitHub Actions
Waiting
WilliamRoebuck
temporarily deployed
to
workflow-approval
July 31, 2026 12:19 — with
GitHub Actions
Inactive
WilliamRoebuck
temporarily deployed
to
workflow-approval
July 31, 2026 12:19 — with
GitHub Actions
Inactive
danth
suggested changes
Jul 31, 2026
danth
left a comment
Member
There was a problem hiding this comment.
Various nitpicks and ramblings. I have seen all the code but did not check the unit tests in much detail yet.
Contributor
|
@WilliamRoebuck @NicolasFussberger @danth @MaciejKaszynski |
WilliamRoebuck
requested a deployment
to
workflow-approval
August 4, 2026 08:07 — with
GitHub Actions
Waiting
WilliamRoebuck
requested a deployment
to
workflow-approval
August 4, 2026 08:07 — with
GitHub Actions
Waiting
WilliamRoebuck
force-pushed
the
feature/process-mon-refactor-2
branch
from
August 4, 2026 09:03
a88ee00 to
683211b
Compare
WilliamRoebuck
requested a deployment
to
workflow-approval
August 4, 2026 09:03 — with
GitHub Actions
Waiting
WilliamRoebuck
requested a deployment
to
workflow-approval
August 4, 2026 09:03 — with
GitHub Actions
Waiting
WilliamRoebuck
requested a deployment
to
workflow-approval
August 4, 2026 09:03 — with
GitHub Actions
Waiting
WilliamRoebuck
marked this pull request as ready for review
August 4, 2026 14:10
WilliamRoebuck
requested review from
FScholPer,
MaciejKaszynski,
anmittag,
antonkri,
pawelrutkaq and
ramceb
as code owners
August 4, 2026 14:10
MaciejKaszynski
previously approved these changes
Aug 5, 2026
danth
reviewed
Aug 5, 2026
WilliamRoebuck
requested a deployment
to
workflow-approval
August 5, 2026 08:46 — with
GitHub Actions
Waiting
WilliamRoebuck
requested a deployment
to
workflow-approval
August 5, 2026 08:46 — with
GitHub Actions
Waiting
WilliamRoebuck
requested a deployment
to
workflow-approval
August 5, 2026 08:46 — with
GitHub Actions
Waiting
WilliamRoebuck
requested a deployment
to
workflow-approval
August 5, 2026 08:46 — with
GitHub Actions
Waiting
WilliamRoebuck
requested a deployment
to
workflow-approval
August 5, 2026 09:04 — with
GitHub Actions
Waiting
WilliamRoebuck
requested a deployment
to
workflow-approval
August 5, 2026 09:04 — with
GitHub Actions
Waiting
WilliamRoebuck
force-pushed
the
feature/process-mon-refactor-2
branch
from
August 5, 2026 09:08
5405417 to
40a2e6e
Compare
WilliamRoebuck
requested a deployment
to
workflow-approval
August 5, 2026 09:08 — with
GitHub Actions
Waiting
WilliamRoebuck
requested a deployment
to
workflow-approval
August 5, 2026 09:08 — with
GitHub Actions
Waiting
WilliamRoebuck
temporarily deployed
to
workflow-approval
August 5, 2026 09:16 — with
GitHub Actions
Inactive
WilliamRoebuck
temporarily deployed
to
workflow-approval
August 5, 2026 09:16 — with
GitHub Actions
Inactive
MaciejKaszynski
approved these changes
Aug 5, 2026
NicolasFussberger
approved these changes
Aug 5, 2026
danth
approved these changes
Aug 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The description below is not AI generated. It includes some key information needed to begin a review.
Overview
This is the completion of the refactor started in #370. With these changes, the following is achieved:
IComponent. This means that different types of component can now be implemented, such as containers.Graphnow processes events sequentially rather than in parallel. SeeComponentEventQueue.ProcessInfoNodeis no longer responsible for dependency resolution. This responsibility is moved toTransition.ProcessInfoNode&Graphcoupling resolved -ProcessInfoNodeno longer has access toGraph.New components
DependencyGraph- Data structure for storing and traversing component dependenciesTransition- Resolves run target activation logic. Activates and deactivates components as required.Behavioral change
Processes are now managed based on component state, rather than process state. This means that once a component is activated, the underlying process shall not be started again until the component becomes inactive (either due to an error or a run target activation). Previously, a self-terminating process would be restarted on every run target activation that included it.
Minor changes
kLaunchManagercomms type removed. This was legacy and no longer needed.Review strategy
As this PR is quite large, it may be necessary for reviewers each to tackle subsections of the changes. In terms of line numbers, these sections are roughly equal:
graph.cpp+graph.hpp(+ unit tests)processinfonode.cpp+processinfonode.hpp(+ unit tests)transition.hpp+transition_UT.hppdependency_graph.hpp+dependency_graph_UT.cpp+process_group_manager.cpp+process_group_manager.hpp