-
Notifications
You must be signed in to change notification settings - Fork 140
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
wip: upgrade stardoc and protobuf to restore Bazel HEAD CI #1462
base: master
Are you sure you want to change the base?
wip: upgrade stardoc and protobuf to restore Bazel HEAD CI #1462
Conversation
alternative approach to bumping up protobuf so much would be to pin to rules_proto 7.0.2 |
@@ -74,7 +76,9 @@ A provider whose type/layout is an implementation detail and should not | |||
## derive_swift_module_name | |||
|
|||
<pre> | |||
derive_swift_module_name(<a href="#derive_swift_module_name-args">args</a>) | |||
load("@rules_swift//doc:doc.bzl", "derive_swift_module_name") |
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.
Ugh, this is annoying.
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.
The main annoying thing is doc.bzl
versus where it actually is 😕.
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.
is this something we should bring up in the stardoc repo as a bug? or maybe there's another way we're supposed to be organizing these?
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.
Could probably use a full issue. I saw it mentioned in a comment here: bazelbuild/stardoc#94 (comment)
I think we need to go the route of concatenating the output instead.
Pros and cons of these approaches? Could the protobuf bump break people that depend on us? |
I'm not super-clued into the goings-on around rules_proto/protobuf, but I saw after writing that comment that stardoc 0.7.2 declares a dep on protobuf 29.0. So I think it might be a moot point. |
we could probably pull in a patch to revert bazelbuild/stardoc#216 to avoid the load issues |
Trying my hand at this, to improve personal competency. This updates the minimum protobuf to 29.0, removes the dependency on rules_proto, and updates stardoc to 0.7.2. Some or all of this may be an overreach, but I'm unfamiliar with the constraints.