-
Notifications
You must be signed in to change notification settings - Fork 23
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
Generated Protocol Bindings #1931
Conversation
// This is just a temporary solution to make it easier to use certain generated protocol | ||
// messages. | ||
// Ultimately this should entirely be replaced by use of the generated protocol. | ||
if (!isForceProtocolCopy) { |
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.
What is the purpose of this flag if it needs to always be set for task to have any effect?
Shouldn't force be necessary only if the protocol files already exists? (is it even an option that no such files would be present)?
If that flag is always needed then equally good we can just not have it.
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'll address this when we set up CI & automatic generation on builds etc. For now it's all manual while we iron out the wrinkles
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 except one other comment.
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.
ok but pls note the Peter's comment
Adds generated protocol bindings for use in #1921
Test plan