update types.proto to match tendermint 0.15.0, and regenerate sources. #33
Conversation
There was a problem hiding this comment.
First: thanks for this!
Second: why did you ommit all those gogo changes in the ABCI protobuf? Aren't they required?
See https://github.com/tendermint/abci/blob/5d5ea6869b91cadb55dbc4211ad7b326f053a33e/types/types.proto#L198 for example
| package com.github.jtendermint.jabci; | ||
|
|
||
| public enum CodeType | ||
| implements com.google.protobuf.ProtocolMessageEnum { |
There was a problem hiding this comment.
Why implementing the protobuf enum?
There was a problem hiding this comment.
Glad to see this. As mentioned, I'm a bit concerned with the lack of gogoproto magic that was introduced in abci/types.proto. See https://github.com/tendermint/abci/blob/5d5ea6869b91cadb55dbc4211ad7b326f053a33e/types/types.proto#L198
Can we clarify what it means to not use those supplementary types?
| @@ -0,0 +1,372 @@ | |||
| package com.github.jtendermint.jabci; | |||
|
|
|||
| public enum CodeType | |||
There was a problem hiding this comment.
Another idea could be to just use a class with constants instead of enum. So we can get rid of all the protobuf things here. In the end, abci/tendermint just expects an INT as code.
There was a problem hiding this comment.
You are right. We can declare a class, rather than using the enum generated by protobuf, which is removed from types.proto now.
|
I find it is difficult to port gogoproto to non go languages, so I removed that. It seems no any side effects. |
|
@longzhi yeah, understood. Let's see what happens. We've opened tendermint/abci#179 to adress those concerns. Have you tried the port on any non-trivial cases? |
|
@longzhi I started over after a discussion in tendermint/abci#179 I would like to close this PR here in favor of #35 - are you OK with that? It would be amazing if you could somehow also test the new branch (tm-15) before we merge. |
|
0.15 released |
No description provided.