-
-
Notifications
You must be signed in to change notification settings - Fork 90
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
Refactor importer initialization #647
Conversation
4351143
to
250b68e
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.
I like where this is headed, and only have a few minor things I'd like to see addressed. It seems to be working nicely.
Besides the inline comments, the only thing that needs to be addressed is the yet missing changelog entry.
Oh, one more thing I forgot to mention during the review: References to elements of structured bindings are not explicitly allowed by the C++17 standard (fixed for C++20, I believe). They happen to work with the gcc version used by CI, but fail to compile with clang8, which is why the CI build is failing on FreeBSD and macOS. |
I changed to code to not make use of structured bindings in |
045b73c
to
cfc7c1e
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.
lgtm
The IMPORTER used to be set up with a 2-step initialization process. Some necessary parameters were passed at the call to
spawn
and others are supplied by message from the TRACKER. This PR moves all essential parameters to the spawning invocation.Additionally, this PR factors the logic of retrieving NODE components into the dedicated functions
get_component(node_actor*)
andget_node_component(scoped_actor&, actor)
to further decouple the TRACKER from the other actors (it is an implementation detail of the NODE and should not leak through the interface).TODO:
get_node_component()
insource_command
.