Another gogo improvement: inline stringmap structs to avoid another u… #64
Conversation
|
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. |
|
CLAs look good, thanks! |
mixer/v1/report.proto
Outdated
| @@ -39,4 +39,7 @@ message ReportResponse { | |||
|
|
|||
| // Indicates whether the report was processed or not | |||
| google.rpc.Status result = 2 [(gogoproto.nullable) = false]; | |||
|
|
|||
| // The attributes to use for this response | |||
| Attributes attribute_update = 3; | |||
There was a problem hiding this comment.
I was my intent to make those all the same, but I guess my mind strayed somewhere along the way.
PTAL.
| // Indicates whether or not the preconditions succeeded | ||
| google.rpc.Status result = 2 [(gogoproto.nullable) = false]; | ||
| google.rpc.Status result = 3 [(gogoproto.nullable) = false]; | ||
|
|
There was a problem hiding this comment.
"2" used to be "result", now it is used for attribute_update. This is bad. It breaks the backward compatibility. You never re-use a proto index.
There was a problem hiding this comment.
Sorry, given the phase the project is in I didn't think backward compat was an issue yet.
There was a problem hiding this comment.
We did this (and in general have been changing field indices) because these protos aren't being persisted anywhere today - the field number change should have no effect on anyone. We're still pre-Alpha so all bets are off w.r.t. backwards compatibility. Post official Alpha release I totally agree this kind of shuffling of fields must never happen.
There was a problem hiding this comment.
It is between proxy and mixer. Without this change, we can safely say all proxy versions works with all mixer versions. Now with this change, we could not say that any more.
The demo users have to be careful which version of proxy and mixer can work together. They are not always using the latest.
This patch imports istio.io API protos (hence the new additional `-I` to include `-Icommon-protos/istio.io/api`).
So, e.g. in `install/controlplane/v1alpha1/spec.proto`,
```proto
import "istio.io/api/operator/v1alpha1/component.proto";
message ControlPlaneSpec {
...
repeated istio.operator.v1alpha1.K8sObjectOverlay overlays = 700;
}
```
Signed-off-by: Dhi Aurrahman <[email protected]>
…seless pointer.