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

Run regenerate_docs.sh #297

Merged
merged 2 commits into from
Sep 24, 2021
Merged

Run regenerate_docs.sh #297

merged 2 commits into from
Sep 24, 2021

Conversation

alexeagle
Copy link
Contributor

@alexeagle alexeagle commented May 11, 2021

Seems that recent edits were done by hand? Is the generator script still functioning?

  • remove flag that no longer exists in recent bazel releases from regenerate_docs script
  • use a less verbose path in the output tree so bazel-bin doesn't look so strange
  • now uses markdown tables since that's what the pinned version of stardoc does

@google-cla google-cla bot added the cla: yes label May 11, 2021
@alexeagle alexeagle changed the title Be consistent about including generated header in generated docs Run regenerate_docs.sh May 12, 2021
@alexeagle
Copy link
Contributor Author

Looks like the problem was reported last time someone touched the docs, but then not followed up?
#280 (comment)

Copy link
Collaborator

@tetromino tetromino left a comment

Choose a reason for hiding this comment

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

Thank you for catching this! Agree with the PR, but I would like to limit it just to regenerating the docs and removing the obsolete --experimental_remap_main_repo flag.


for filename in bazel-bin/docs/*_gen.md; do
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would prefer to avoid changing the rename logic here (and thus avoid changing all the outs in docs/BUILD) without a specific reason. The existing logic works, and "_gen" suffix in a generated doc file is a good signal.

docs/BUILD Outdated
@@ -6,132 +6,132 @@ licenses(["notice"])
# the --incompatible_remap_main_repo flag.
stardoc(
name = "build_test_docs",
out = "build_test_doc_gen.md",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't see a good reason for the out renames here and below in docs/BUILD. "_gen" means a generated, not hand-written md doc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, I'll open a separate PR for that change and discuss my reasoning.

Seems that recent edits were done by hand? Is the generator script still functioning?

- remove flag that no longer exists in recent bazel releases from regenerate_docs script
- now uses markdown tables since that's what the pinned version of stardoc does
@alexeagle
Copy link
Contributor Author

@tetromino is the intent for the docs to always be up-to-date? If so, the root cause here was a missing diff_test which I could add (similar to recently added to rules_apple for instance https://github.com/bazelbuild/rules_apple/blob/master/doc/BUILD.bazel#L36-L45 )

@alexeagle
Copy link
Contributor Author

ping @tetromino would you like tests added here that assert the docs are always up-to-date?

@tetromino
Copy link
Collaborator

@alexeagle - yes, a test to check docs are up to date would be greatly appreciated (as a follow-up PR).

Copy link
Collaborator

@tetromino tetromino left a comment

Choose a reason for hiding this comment

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

Thanks, looks good!

There are some conflicts reported - let me see if they can be fixed trivially...

@tetromino tetromino merged commit eabe5f7 into bazelbuild:main Sep 24, 2021
alexeagle added a commit to alexeagle/bazel-skylib that referenced this pull request Oct 9, 2021
it prints a convenient 'bazel run' command to update them, replacing the shell script

Follow-up to bazelbuild#297 (comment)
alexeagle added a commit to alexeagle/bazel-skylib that referenced this pull request Oct 9, 2021
it prints a convenient 'bazel run' command to update them, replacing the shell script

Follow-up to bazelbuild#297 (comment)
alexeagle added a commit to alexeagle/bazel-skylib that referenced this pull request Oct 9, 2021
it prints a convenient 'bazel run' command to update them, replacing the shell script

Follow-up to bazelbuild#297 (comment)
alexeagle added a commit to alexeagle/bazel-skylib that referenced this pull request Oct 9, 2021
it prints a convenient 'bazel run' command to update them, replacing the shell script

Follow-up to bazelbuild#297 (comment)
@alexeagle alexeagle deleted the docs branch October 20, 2021 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants