-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Fix regression in regenerating protobuf source #8603
Conversation
@alamb The use of the |
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.
Thanks @andygrove
@@ -1338,7 +1338,11 @@ pub fn parse_expr( | |||
in_list.negated, | |||
))), | |||
ExprType::Wildcard(protobuf::Wildcard { qualifier }) => Ok(Expr::Wildcard { | |||
qualifier: qualifier.clone(), | |||
qualifier: if qualifier.is_empty() { |
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 fine -- I don't think an empty string ""
is a valid qualifier anyways so not being able to distinguish between ""
and None
is not important
Sorry for the late comment. I think it would be great if we align the version in the docs with the version used by CI/CD pipeline. Will submit a PR later |
Which issue does this PR close?
Closes #8602
Rationale for this change
Fix regression in generating protobuf source with protobuf 3.12, which is the recommended version in the documentation.
What changes are included in this PR?
Remove redundant
optional
keyword.Are these changes tested?
Existing tests.
Are there any user-facing changes?