search

googleapis / proto-breaking-change-detector

Apache License 2.0
18 stars 8 forks source link

Improve LRO response and metadata types comparison #226

Open alexander-fenster opened 2 years ago

alexander-fenster commented 2 years ago

Right now, LRO response and metadata types are only compared by name:

https://github.com/googleapis/proto-breaking-change-detector/blob/597343e8ef09f96c60d3274789f82b7dee76512a/src/comparator/service_comparator.py#L386-L390

https://github.com/googleapis/proto-breaking-change-detector/blob/597343e8ef09f96c60d3274789f82b7dee76512a/src/comparator/service_comparator.py#L408-L412

There can be a case when the type is changed to its fully qualified name (OperationMetadata to google.cloud.example.v1.OperationMetadata), which is the same type, but the BCD will return an error in this case. I would like to have this fixed.

Note that changing the type to semantically equivalent but different type (google.cloud.example.v1.OperationMetadata to google.cloud.another_example.v1.OperationMetadata) is indeed a breaking change, even if the types are exactly similar. So this issue is only about the cases where the type name is changed from short to fully qualified, or vice versa.

tmatsuo commented 2 years ago

If there's a canonical way of converting short name to the fully qualified type name, we can always compare the fully qualified names.

alexander-fenster commented 2 years ago

As I understand it, the canonical way is to prepend the package name of the given proto file. We might need to pass it down to the method comparator, I'm not sure if it's accessible from the comparator at this moment.