Make protobuf-ts as conformant as protobuf-es #567
Merged
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
timostamm
approved these changes
Jul 31, 2023
There was a problem hiding this comment.
Thank you 🙂
would it make more sense to just add the packages/test-conformance/src/gen folder to the .gitignore?
I've come around to prefer generated code to be checked in, especially for code generator projects. It means every commit contains the full picture. What we are still missing here is CI checking for diffs.
This was referenced May 24, 2024
This was referenced Jul 19, 2024
This was referenced Jul 26, 2024
This was referenced Aug 10, 2024
This was referenced Sep 11, 2024
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 change fixes all remaining conformance tests except for
Recommended.Proto2.JsonInput.FieldNameExtension.Validator(and obviously all the text format ones). This brings protobuf-ts up to the same level of conformance as protobuf-es.The reason that the
Required.Proto3.JsonInput.EnumFieldWithAliasLowerCaseandRequired.Proto3.JsonInput.EnumFieldWithAliasUseAliasconformance tests were failing was because the test_messages_proto3.proto file hadn't been updated to match the version of the conformance runner. It has been updated (along with the test_messages_proto2.proto and conformance.proto files) to match what's in v22.3.Side note: would it make more sense to just add the
packages/test-conformance/src/genfolder to the.gitignore? It'd make this PR a lot easier to grok.