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 reserved extension declaration to specify full_name and type as long as they specify both #327

Merged

Conversation

jhump
Copy link
Member

@jhump jhump commented Aug 7, 2024

The logic was incorrect and failed to mirror protoc. The test case, which verifies that protoc succeeds or fails when protocompile does, was not correctly formulated so gave false confidence that the logic was correct.

Previously, this disallowed extension declarations that were reserved that also had a full_name or type attribute specified.

With this change, it is allowed as long as both full_name and type are specified. It is only an error when only one of them is set.

@jhump jhump requested a review from srikrsna-buf August 7, 2024 14:52
@jhump
Copy link
Member Author

jhump commented Aug 7, 2024

cc @mkruskal-google

@mkruskal-google
Copy link

Thanks Josh!

@jhump jhump merged commit 1c07140 into main Aug 7, 2024
9 checks passed
@jhump jhump deleted the jh/extension-decls-can-have-name-and-type-when-reserved branch August 7, 2024 18:59
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.

3 participants