Skip to content
This repository was archived by the owner on Jan 14, 2026. It is now read-only.

Comments

docs: begin generating docs for defs#186

Merged
copybara-service[bot] merged 8 commits intobazelbuild:mainfrom
thesayyn:docs
Oct 26, 2023
Merged

docs: begin generating docs for defs#186
copybara-service[bot] merged 8 commits intobazelbuild:mainfrom
thesayyn:docs

Conversation

@thesayyn
Copy link
Collaborator

No description provided.

@thesayyn thesayyn requested review from a team and comius as code owners October 25, 2023 19:30
@thesayyn
Copy link
Collaborator Author

i don't know why rules_license is needed all of a sudden...

@comius
Copy link
Collaborator

comius commented Oct 26, 2023

i don't know why rules_license is needed all of a sudden...

Borked up internal setup. I fixed it, however I was waiting for internal review of the fixes. Now they are submitted, rules_proto again pass at head. So you can drop the license related modification.

comius
comius previously approved these changes Oct 26, 2023
@comius
Copy link
Collaborator

comius commented Oct 26, 2023

LGTM.

I dislike that the docs/stardoc_with_diff_test.bzl is copied and not public in skylib. But it looks like copying is the best of options.

@comius comius self-requested a review October 26, 2023 17:03
@comius comius added the ready to pull Required label for automatic import to Google label Oct 26, 2023
comius
comius previously approved these changes Oct 26, 2023
@thesayyn
Copy link
Collaborator Author

yes, we have that as part of our bazel-lib at aspect, i guess i can't add that as a dev dependency here?

@comius
Copy link
Collaborator

comius commented Oct 26, 2023

yes, we have that as part of our bazel-lib at aspect, i guess i can't add that as a dev dependency here?

No please don't. I'd then need to pull it into google or rewrite it.

@comius comius removed the ready to pull Required label for automatic import to Google label Oct 26, 2023
bellspice
bellspice previously approved these changes Oct 26, 2023
comius
comius previously approved these changes Oct 26, 2023
@comius comius added the ready to pull Required label for automatic import to Google label Oct 26, 2023
@comius comius added ready to pull Required label for automatic import to Google and removed ready to pull Required label for automatic import to Google labels Oct 26, 2023
file1 = out_label,
# Output from stardoc rule above
file2 = out_file.replace(".md", "-docgen.md"),
tags = ["no_windows"],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test is broken on the import. That's because we use native.starlark_doc_extract everywhere. The latter, available in Bazel, will also generate the documetation for proto_common builtin to Bazel (because it refers to it directly).

Usually a quick fix would be to disable the test using a tag. However I can't do this here, because the stardoc_with_diff_test doesn't pass tags to the difftest.

For a quick solution I'll can comment out the test on the import.

Alternative would be that you also use native.starlark_doc_extract. But then you'd need to update the docs on each Bazel release. Which doesn't seem so bad. Also proto_common documentation is not available anywhere I think. (So maybe it makes sense to do on a followup PR)

@comius comius added ready to pull Required label for automatic import to Google and removed ready to pull Required label for automatic import to Google labels Oct 26, 2023
@copybara-service copybara-service bot merged commit 52b8044 into bazelbuild:main Oct 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

ready to pull Required label for automatic import to Google

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants