-
Notifications
You must be signed in to change notification settings - Fork 5
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
Improve typing #102
base: main
Are you sure you want to change the base?
Improve typing #102
Conversation
Because we know which bytes exist via its length but sorbet can't infer it
Since the implementation currently has a bug that does not type check, which actually surfaced the bug.
426d982
to
1cd4c74
Compare
Since we always encode the default value
"required" and "optional" refer to the encoding. If a field is "required", it means that it will always be encoded. If it's "optional", then it's only encoded if it is not the default value. (This is confusing from a user's perspective, but unfortunately that's how the spec is defined) I think only fields that contain submessages are actually nillable. Everything else should require some kind of value. |
Interesting. |
@TobiasBales Is this PR something you're still interested in? I accidentally overlooked it when we resumed development on Protoboeuf. There are a few conflicts, but they might be easy to resolve. Did you happen to make any progress with Aaron's feedback? If we're going to merge this, doing so soon would be best before ongoing development causes the branch to fall more out of date. |
This bumps the # typed sigil to strict and runs
srb tc
in test cases.This means some adjustments to the generated code (or quite a lot).
I would like to get initial agreement on the direction and then would do some clean up (there are definitely some simplifications possible and required).
Some things/questions/observations that come to mind: