WIP for discussion: Custom descriptor options #280
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- add SUPPORT_PATH, PROTOS_PATH helpers in specs
Custom field / method options can be added by extending the base proto descriptor
enum names are not required to be Capitalized, but ruby constants are. Add option to UPCASE all enum names
nerdrew
force-pushed
the
custom-descriptor-options
branch
from
70b136b to
641f788
Compare
|
I think the best path forward is the one that keeps the potentially dangerous changes together so that the maintainers can focus on the PRs that matter most. I suggest:
|
|
Sounds Good. I'm going to trickle the PRs in so I don't have to rebase all over the place: #281 (fix rspec warnings) |
|
Fix for |
|
Consolidate, cleanup, and refresh protos from upstream: #283 |
|
Add missing deprecation requires: #284 |
|
First feature PR, add |
|
Closing this PR for now. Will open a new PR for the custom options / extensions after the others are merged. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a large PR with a ton of stuff jumbled together, but I have questions about how to slice it.
Objections to moving all the test protos to
spec/support/protos? This would include*.proto,*.pb.rb, and*.binfiles (proto file, generated code, and raw proto bytes).1 PR with a bunch of unrelated commits (e.g. 1 commit to cleanup
raise_errorwarnings, 1 commit with an option to upcase enum names, 1 commit to move spec files, etc) vs a PR / commit?Thoughts on adding a flag for generating gRPC specific service stubs?
Note: one reason this PR appears so large is that I refreshed some of the google unittest.proto files from upstream (will break that into either its own commit or its own PR pending question 2 above).
cc @lukaszx0