Uh oh!
There was an error while loading. Please reload this page.
revert: upgrade to protobuf 27.0 and remove py_proto_library (#1933) - #1948
Merged
rickeylev merged 1 commit intoJun 11, 2024
Merged
Conversation
…ontrib#1933) This reverts commit d0e25cf. We have the following concerns with the associated change: - `sphinxdocs` is excluded entirely. Because protobuf fails to compile? But why? Protos are needed as part of our docgen, but it is unclear how the docgen is still working. Maybe we can get some clarification here? - The `py_proto_library` tests in the `bzlmod` example are excluded from CI. Because protos also fail to compile? But why? It's an example and should Just Work. - Adding the `copts` needs to be done by all downstream users now. But fixing that in protobuf blocks removing legacy struct providers? I don't understand the connection. Also @alexeagle noted extra [regression](bazel-contrib#1933 (comment)) being caused by the associated PR. Reverting in order to unblock a new release of `rules_python` and then we can work together with @comius on reverting the revert.
rickeylev
approved these changes
Jun 11, 2024
This file contains hidden or 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
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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 reverts commit d0e25cf.
We have the following concerns with the associated change:
sphinxdocsis excluded entirely. Because protobuf fails to compile?But why? Protos are needed as part of our docgen, but it is unclear
how the docgen is still working. Maybe we can get some clarification
here?
py_proto_librarytests in thebzlmodexample are excluded fromCI. Because protos also fail to compile? But why? It's an example and
should Just Work.
coptsneeds to be done by all downstream users now. Butfixing that in protobuf blocks removing legacy struct providers? I
don't understand the connection.
Also @alexeagle noted extra
regression
being caused by the associated PR.
Reverting in order to unblock a new release of
rules_pythonand thenwe can work together with @comius on reverting the revert.
Reverts #1933