-
Notifications
You must be signed in to change notification settings - Fork 579
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DependencyGraph: use ConfigObject*, not Object* #10264
Conversation
5547c12
to
af064c8
Compare
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.
- To be clear, this is not a requirement for Fix broken downtime comment sync #10000. But as you re-built the DG there and will re-work that due to DependencyGraph: switch "parent" and "child" terminology #10263 anyway, I think it makes sense to make the DG proper right away. Unless, of course, you don't like this PR.
|
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.
If this PR is just about replacing Object*
with ConfigObject*
, why did you make so many unrelated changes? For example, why did you replace std::map
with std::unordered_map
, knowing that this will be changed with #10000 anyway? If you want me to review this, then please drop all the unrelated changes that are not related to what the PR title advertises.
af064c8
to
3022d0c
Compare
I've removed the top commit. (af064c8) |
6db2662
to
78c68b4
Compare
78c68b4
to
5e81b4d
Compare
5e81b4d
to
6db2662
Compare
This saves dynamic_cast<ConfigObject*> + if() on every item of GetChildren().
6db2662
to
3a09cf7
Compare
This saves dynamic_cast<ConfigObject*> + if() on every item of GetChildren().
TODO