Skip to content

Docs: Update PR template to ask for user-visible rationale #24053

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.

Sign up for GitHub

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

Merged
alamb merged 1 commit into from
Aug 4, 2026
+9 −4
Merged

Docs: Update PR template to ask for user-visible rationale #24053

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/ISSUE_TEMPLATE/feature_request.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ body:
description: Please describe what you are trying to do.
placeholder: >
A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]
(This section helps Arrow developers understand the context and *why* for this feature, in addition to the *what*)
(This section helps DataFusion developers understand the context and *why* for this feature, in addition to the *what*)

@alamb alamb Aug 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

drive by fix

- type: textarea
attributes:
label: Describe the solution you'd like
Expand Down
11 changes: 8 additions & 3 deletions .github/pull_request_template.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,19 @@ We generally require a GitHub issue to be filed for all bug fixes and enhancemen
<!--
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.

Please explain the problem you are trying to solve in terms of the user-visible
behavior, rather than the implementation.

For example, "The code in `foo.rs` doesn't handle nulls" is a symptom of the
implementation. "COUNT(DISTINCT) returns wrong results when the column contains
nulls" is the user-visible problem.

@2010YOUY01 2010YOUY01 Aug 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
nulls" is the user-visible problem.
A PR rationale should present a clear chain of reasoning. This helps other contributors understand the intent end to end and engage at any step—for example, by questioning an assumption or proposing an alternative approach.
When a PR description contains only low-level implementation details, reviewers must reconstruct the original intent from the code changes themselves.
Here are the typical steps for describing a bug-fix PR:
1. What user-visible problem are you trying to solve?
Please explain the problem you are trying to solve in terms of the user-visible behavior, rather than the implementation.
For example, "The code in `foo.rs` doesn't handle nulls" is a symptom of the implementation. "COUNT(DISTINCT) returns wrong results when the column contains nulls" is the user-visible problem.
2. What is the high-level idea behind the fix?
3. (In the next 'changes' section) What are the key changes in the PR?
Highlight the changes that are central to the solution. For a large PR, distinguish the core changes from follow-up or propagated fixes. This makes the PR easier to navigate and review.

I think we have very similar ideas, but I tried to explain in more detail. It may be a bit verbose, so perhaps we can make it more concise. 🤔 WDYT?

@alamb alamb Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think this is a bit too verbose for every PR -- how about we add this backstory / content to https://datafusion.apache.org/contributor-guide/index.html#creating-pull-requests and then add a link to that section here?

@2010YOUY01 2010YOUY01 Aug 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sounds good!

-->

## What changes are included in this PR?

<!--
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
There is no need to duplicate the description in the issue here, but it is sometimes worth providing a summary of the individual changes in this PR.
-->

## Are these changes tested?
Expand All @@ -33,8 +40,6 @@ If tests are not included in your PR, please explain why (for example, are they

<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->

<!--
If there are any breaking changes to public APIs, please add the `api change` label.
-->
Loading