-
Notifications
You must be signed in to change notification settings - Fork 665
Add message and enum type information to package definition output. #703
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
Conversation
assert(isTypeObject(packageDefinition.SequenceValues)); | ||
if(isTypeObject(packageDefinition.SequenceValues)) { | ||
const sequenceValuesDef: TypeDefinition = packageDefinition.SequenceValues; | ||
assert.strictEqual(sequenceValuesDef.format, 'Protocol Buffer 3 DescriptorProto'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am kind of wondering if we don't want a bit deeper comparisons here to validate the contents of what is returned? At least maybe not just .format
, but .type
as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a good idea, but it's a significant amount of work so we should do it at a later date. The type
fields are just defined as object
in TypeScript, so it does not like me looking at specific fields of those objects. The best solution for that is to probably write up full TypeScript definitions for those objects, but that is a lot of work and I would like to get this change usable first.
LGTM otherwise, after we can see tests are green thanks to #705. |
The Windows build timed out as usual, and the Linux Build failed with what looks like a CI flake. Rerunning the tests to be sure. |
This is an implementation of gRFC proposal L43, adding message and enum type information to the output of the
load
andloadSync
functions. When loaded into a gRPC implementation usingloadPackageDefinition
those objects will appear in the expected place in the object tree. They will also be included in therequestType
andresponseType
message properties.This supersedes PRs #408 and #448, and fixes #407.