-
Notifications
You must be signed in to change notification settings - Fork 63
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
Port org.lflang.federated
to java
#950
Conversation
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.
This looks good to me, but let's have @Soroosh129 approve this, too.
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.
The port overall looks good to me.
For some reason, GitHub is not showing me the ported FedLauncher file in a diff, so it is hard to verify that the logic matches the old Xtend implementation. I suspect the remote scripts in FedLauncher were broken even before this port, since we have no tests for them. However, just to be sure, would you please try to set the RTI and a federate to a remote host (e.g., 127.0.0.1 should work) and see if the _distribute
and launch scripts work?
You would have to have password-less ssh set up on your local machine (so that ssh 127.0.0.1
doesn't ask for a password, or to store any keys).
At some point, I think we either need to remove these remote scripts and go all in to Docker, or we should add tests for these to make sure they are not broken, but this is obviously beyond the scope of this PR.
The distribute script is indeed broken before the port. The problem is that |
see #838