-
Notifications
You must be signed in to change notification settings - Fork 97
Remove redundant field. #713
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
Conversation
|
bumpity bump bump... @ljbade can you confirm that this is what you initially intended? @silverjam - if you don't have the bandwidth to auto-gen I can take a shot at dockerizing it; lemme know... |
|
@martin-swift updated |
| uint32 update_interval = 5; | ||
| uint32 iod_ssr = 6; | ||
| uint32 update_interval = 4; | ||
| uint32 iod_ssr = 5; |
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.
yow:
Non-required fields can be removed, as long as the field number is not used
again in your updated message type. You may want to rename the field
instead, perhaps adding the prefix "OBSOLETE_", or make the field number
reserved, so that future users of your .proto can't accidentally reuse the number.
So if we rev a message that people are already using we can't delete a field unless we don't care about backwards compatibility?
|
thanks @silverjam ; anyone know what's up with the travis failures? |
|
Since this isn't really published or used externally yet I don't think it
matters that we're deleting a field.
The only risk here is if a consumer of these messages uses a version that's
out of sync. Since we are in control of all these consumers the risk is
low.
External consumers of these messages are far enough away that this probably
isn't very much of a risk.
|
|
Travis failure is a hopefully transient download issue.
|
@silverjam Can you do the honors again for the autogen?