vss-tools icon indicating copy to clipboard operation
vss-tools copied to clipboard

Protobuf use static uid

Open eriksven opened this issue 10 months ago • 4 comments

This PR extends the protobuf generator to use the static-uid which would be created in a previous step with vspec2id generator. The PR also adds the feature to make all fields in the generated protobuf file to optional.

Using the static-uids has some drawbacks but the previously existing approach of using incremental field identifiers has some issue regarding backwards compatibility. Because of that I want to propose the alternative solution of using the static-uids. More details on the advantages and drawbacks of both approaches are available in the new vspec2proto.md Documentation.

eriksven avatar Apr 04 '24 16:04 eriksven

MoM:

  • Please Review

erikbosch avatar Apr 09 '24 14:04 erikbosch

@eriksven I'm not sure if directly comparing static IDs with field numbers at the end makes sense. If we're using static IDs, the message description would likely be different. Additionally, I'm uncertain if we should describe any aspect of VSS in that area.

adobekan avatar Apr 15 '24 13:04 adobekan

MoM:

  • Please review, possible merge decision next week

erikbosch avatar Apr 30 '24 14:04 erikbosch

@adobekan @eriksven - do you intend to participate in the meeting tomorrow (Tuesday 7th)? I think it would be good to agree on if we see any problems with this PR. My view is that this an option that does not affect default behavior, so if someone thinks this is a useful addition it is ok for me

erikbosch avatar May 06 '24 13:05 erikbosch

@adobekan @eriksven - do you intend to participate in the meeting tomorrow (Tuesday 7th)? I think it would be good to agree on if we see any problems with this PR. My view is that this an option that does not affect default behavior, so if someone thinks this is a useful addition it is ok for me

Unfortunately, I am not able to join the meeting today. But I could join one of the next meetings.

eriksven avatar May 07 '24 13:05 eriksven

MoM:

  • Adnan - If I would use static id I would not this approach, it would be more useful if one would use generic message. Would be good to have SvenErik available for a discussion.
  • Erik: Could be useful if server use 4.2, client 4.1
  • Adnan: But then we could use a thin API (id+value), no need for fat API, but both approach valid, but we need to document both options.
  • Erik: Possibly discuss next week
  • Gunnar: Could it help if @eriksven describe intention with change better
  • Erik: discuss next week if this one needs to be part of 4.2

erikbosch avatar May 07 '24 14:05 erikbosch

MoM:

  • SvenErik presented a bit on the background, using staticuid gives an opportunity to generate messages
  • Erik: Do not really see any blockers, it is optional, could be useful in some scenarios
  • Merge

erikbosch avatar May 14 '24 14:05 erikbosch