Skip to content

Rewrite Rules API docs for call-by-name.#22699

Merged
benjyw merged 3 commits intopantsbuild:mainfrom
benjyw:update_docs
Sep 25, 2025
Merged

Rewrite Rules API docs for call-by-name.#22699
benjyw merged 3 commits intopantsbuild:mainfrom
benjyw:update_docs

Conversation

@benjyw
Copy link
Copy Markdown
Contributor

@benjyw benjyw commented Sep 24, 2025

No description provided.

@benjyw benjyw added documentation release-notes:not-required [CI] PR doesn't require mention in release notes labels Sep 24, 2025
@benjyw benjyw requested review from sureshjoshi and tdyas September 24, 2025 21:57
@benjyw
Copy link
Copy Markdown
Contributor Author

benjyw commented Sep 24, 2025

Reviewers: I recommend starting with docs/docs/writing-plugins/the-rules-api/concepts.mdx and then looking at docs/docs/writing-plugins/the-rules-api/union-rules-advanced.mdx. Those are the most substantial rewrites. The diffs are probably not instructive.

Then the remaining files are almost entirely just changing examples to use call-by-name.

Comment thread docs/docs/writing-plugins/the-rules-api/concepts.mdx Outdated
Comment thread docs/docs/writing-plugins/the-rules-api/concepts.mdx Outdated
Comment thread docs/docs/writing-plugins/the-rules-api/concepts.mdx Outdated
Comment thread docs/docs/writing-plugins/the-rules-api/concepts.mdx Outdated
) -> ListGoal:
from pants.engine.rules import rule
from pants.engine.intrinsics import execute_process
from pants.engine.process import fallible_to_exec_result_or_raise
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
from pants.engine.process import fallible_to_exec_result_or_raise
from pants.engine.process import execute_process_or_raise

Comment thread docs/docs/writing-plugins/the-rules-api/concepts.mdx Outdated
Comment thread docs/docs/writing-plugins/the-rules-api/concepts.mdx Outdated
Comment thread docs/docs/writing-plugins/the-rules-api/concepts.mdx Outdated
Comment thread docs/docs/writing-plugins/the-rules-api/concepts.mdx
Comment thread docs/docs/writing-plugins/the-rules-api/concepts.mdx
Comment thread docs/docs/writing-plugins/the-rules-api/concepts.mdx
Comment thread docs/docs/writing-plugins/the-rules-api/concepts.mdx
Comment thread docs/docs/writing-plugins/the-rules-api/concepts.mdx
Comment thread docs/docs/writing-plugins/the-rules-api/concepts.mdx Outdated
Comment thread docs/docs/writing-plugins/the-rules-api/installing-tools.mdx Outdated
Comment thread docs/docs/writing-plugins/the-rules-api/installing-tools.mdx Outdated
Comment thread docs/docs/writing-plugins/the-rules-api/processes.mdx Outdated
Comment thread docs/docs/writing-plugins/the-rules-api/tips-and-debugging.mdx Outdated
Comment thread docs/docs/writing-plugins/the-rules-api/union-rules-advanced.mdx Outdated
Comment thread docs/docs/writing-plugins/the-rules-api/migrating-gets.mdx Outdated
Comment thread docs/docs/writing-plugins/the-rules-api/migrating-gets.mdx Outdated
Comment thread docs/docs/writing-plugins/the-rules-api/migrating-gets.mdx Outdated
Comment thread docs/docs/writing-plugins/the-rules-api/migrating-gets.mdx Outdated
Comment thread docs/docs/writing-plugins/the-rules-api/migrating-gets.mdx Outdated
Comment thread docs/docs/writing-plugins/the-rules-api/migrating-gets.mdx
Comment thread docs/docs/writing-plugins/the-rules-api/migrating-gets.mdx
Copy link
Copy Markdown
Member

@sureshjoshi sureshjoshi left a comment

Choose a reason for hiding this comment

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

Thanks for doing this metric ton of work!

I went through and made some nits, clarifications, and a few other things.

General feedback was of the form "we should immediately let the user know that we'll be revisiting a confusing topic/idea below"

If we want to bikeshed on any of this, I'm still approving - as the results here are better and more valuable than what we have online, so we could always push feedback to a follow-up PR.

@benjyw
Copy link
Copy Markdown
Contributor Author

benjyw commented Sep 25, 2025

Thanks for the very useful comments and suggestions @sureshjoshi ! I've addressed them all.

@benjyw
Copy link
Copy Markdown
Contributor Author

benjyw commented Sep 25, 2025

Except I kept fallible_to_exec_result_or_raise as-is in that one example, since the reader had encountered it earlier.

If we do kill that name entirely we can revisit.

@benjyw benjyw merged commit 325a83b into pantsbuild:main Sep 25, 2025
24 checks passed
@benjyw benjyw deleted the update_docs branch September 25, 2025 19:35
Copy link
Copy Markdown
Member

@cognifloyd cognifloyd left a comment

Choose a reason for hiding this comment

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

I know this is already merged. I wanted to read through anyway to see the change diffs. Very nice.

I noticed 2 minor oddities that, of course, can only be fixed in a separate PR.

Comment thread docs/docs/writing-plugins/the-rules-api/concepts.mdx
@benjyw
Copy link
Copy Markdown
Contributor Author

benjyw commented Oct 5, 2025

Fixed in #22733

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation release-notes:not-required [CI] PR doesn't require mention in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants