Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Protobuf API V2: finish implementation and switchover skycfg to using new impl #93

Merged
merged 8 commits into from
Aug 19, 2021

Conversation

seena-stripe
Copy link
Collaborator

This PR is currently stacked on top of #90, please review by commit

Summary

This PR completes the implementation on the new go protobuf v2 api, and makes the cutover to using the new implementation (points skycfg.go at go/protomodule, deletes internal/go/skycfg).

Specifically this pr

  • Changes protomodule.go's global functions (e.g. proto.clear) to operate on the new *protoMessage object rather than the old message type.
  • Adds package to protomodule and finishes implementing message_type so that pkg = proto.package("skycfg.test_proto") will return an object representing the package, and then pkg.MessageV3 returns an object (protoMessageType) (that can construct a new *protoMessage with MessageV3
  • With global functions, package and message_type working, this PR adds tests on the message object and global functions
  • Deletes internal/go/skycfg, but moves internal/go/skycfg/skycfg_test.go to the top level directory
  • Updates skcyfg.go to use the new go/protomodule

Test

This PR adds the original internal/go/skycfg/proto_test.go and internal/go/skycfg/skycfg_test.go to testing the new implementation

I have also tested this internally on our existing usage of skycfg, snapshotting before and after the implementation switch and confirming there are no differences in output (at least in our usage)

@seena-stripe seena-stripe force-pushed the seena/protobuf-api-v2 branch 4 times, most recently from 03cc116 to d86f339 Compare August 19, 2021 15:39
@seena-stripe seena-stripe merged commit bf4da95 into trunk Aug 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants