Skip to content

Conversation

@se7entyse7en
Copy link
Contributor

@se7entyse7en se7entyse7en commented Feb 14, 2019

First task from issue: src-d/lookout#83.

@carlosms
Copy link
Contributor

Looks like the code generated by you and by travis is different. Maybe there is a problem with the versions used.

@se7entyse7en
Copy link
Contributor Author

@carlosms you're right, the version was wrong. Thanks!

@carlosms
Copy link
Contributor

Maybe we should make the field reserved?

@se7entyse7en
Copy link
Contributor Author

@carlosms I don't completely understand what is the case that would cause problems. AFAIU the problem arises if now we remove it and then let's say next week we add another field with the same name and number, but for example a different type. But that could be a problem if that object is interpreted using the old version of the proto file right?

@smacker
Copy link
Contributor

smacker commented Feb 14, 2019

imo we are still at the very early stage of lookout and pollute it with reserved doesn't make much sense to me.

@carlosms
Copy link
Contributor

🤷‍♂️ ok let's skip the reserved

@se7entyse7en se7entyse7en merged commit 5b98f1e into src-d:master Feb 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants