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

Allow *.orion.pb.go files to access the exported-service-desc from *.pb.go #187

Closed
wants to merge 7 commits into from

Conversation

cs-lexliu
Copy link
Contributor

@cs-lexliu cs-lexliu commented Sep 12, 2022

Allow *.orion.pb.go files to access the exported-service-desc from *.pb.go

Add two execution parameters.

  • standalone-mode: Allow you to place *.orion.pb.go and *.pb.go into different folder.
  • exported-service-desc: Allow you to use the higher version (v1.20.0) of protoc-gen-go.

Read go_package option from protobufs files.
In standalone mode, the external import path is getting from the go_package option.

#186

The generate method has a side-effect on input data.
Add a return value for generate method to prevent the data to be changed within the method.
Decouple the dependency between generator logic and proto lib.
Create a new generator package to make the logic can be reuse by other main function.
rename orion.go to main.go, main is a standard name for the main function files.
…pb.go

Add two execution parameters.
`standalone-mode`: Allow you to place *.orion.pb.go and *.pb.go into different folder.
`exported-service-desc`: Allow you to use the higher version (v1.20.0) of protoc-gen-go.

Read go_package option from protobufs files.
In standalone mode, the external import path is getting from the go_package option.
@codecov
Copy link

codecov bot commented Sep 12, 2022

Codecov Report

Merging #187 (1195658) into master (cdcf61b) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #187   +/-   ##
=======================================
  Coverage   58.49%   58.49%           
=======================================
  Files          13       13           
  Lines         559      559           
=======================================
  Hits          327      327           
  Misses        225      225           
  Partials        7        7           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@@ -0,0 +1,122 @@
package inputs
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inputs package decouples the dependency of protobuf library from internal logic.

@@ -0,0 +1,82 @@
module github.com/carousell/Orion/protoc-gen-orion/internal/testprotos
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Introduce a internal/testprotos sub-module for isolating the github.com/carousell/Orion dependency.
With this file, protoc-gen-orion wouldn't need to depend on github.com/carousell/Orion.

//
// The generated code is documented in the package comment for
// the library.
package main
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keep the main function in protoc-gen-orion/main.go. And move the generating logic into internal/generator/generator.go file.

@benjaminheng
Copy link
Contributor

I'll review it soon. I expected only a few changes to add the flag handling. The large refactor means I'll need to set aside a larger block of time, which I don't have yet.

@cs-lexliu cs-lexliu marked this pull request as draft September 22, 2022 03:52
@cs-lexliu
Copy link
Contributor Author

The feature is implemented in another PR without the refactoring.
#189

@cs-lexliu cs-lexliu closed this Sep 27, 2022
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.

Allow *.orion.pb.go files to be generated into a different package from the generated protobufs
2 participants