search

src-d / lookout-sdk

SDK for lookout analyzers
Apache License 2.0
4 stars 11 forks source link

Removes merge field from ReviewEvent and regenerates files #74

Closed se7entyse7en closed 5 years ago

se7entyse7en commented 5 years ago

First task from issue: https://github.com/src-d/lookout/issues/83.

carlosms commented 5 years ago

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

se7entyse7en commented 5 years ago

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

carlosms commented 5 years ago

Maybe we should make the field reserved?

se7entyse7en commented 5 years ago

@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 commented 5 years ago

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

carlosms commented 5 years ago

:man_shrugging: ok let's skip the reserved