Skip to content

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

Merged
alamb merged 1 commit into
apache:mainfrom
alamb:alamb/update_pr_template
Aug 4, 2026
Merged

Docs: Update PR template to ask for user-visible rationale#24053
alamb merged 1 commit into
apache:mainfrom
alamb:alamb/update_pr_template

Conversation

@alamb

@alamb alamb commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

PR descriptions are most useful when they describe the problem being solved from the user's point of view, rather than describing what is wrong with some part of the code.

What changes are included in this PR?

  1. Update .github/pull_request_template.md to ask authors to explain the problem in terms of user-visible behavior (with an example), and to add the api change label for breaking public API changes.

Are these changes tested?

No tests needed: template-only change.

Are there any user-facing changes?

No changes to the code or documentation; contributors will see the updated
templates when opening PRs and feature requests.

@github-actions github-actions Bot added the development-process Related to development process of DataFusion label Aug 2, 2026
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*)

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

@alamb
alamb marked this pull request as ready for review August 2, 2026 11:12
@alamb alamb added the documentation Improvements or additions to documentation label Aug 2, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.85%. Comparing base (66c3840) to head (3020d0b).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #24053      +/-   ##
==========================================
- Coverage   80.86%   80.85%   -0.01%     
==========================================
  Files        1101     1101              
  Lines      375446   375446              
  Branches   375446   375446              
==========================================
- Hits       303592   303585       -7     
- Misses      53761    53768       +7     
  Partials    18093    18093              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.


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.

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?

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?

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!

@alamb
alamb added this pull request to the merge queue Aug 4, 2026
@alamb

alamb commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @2010YOUY01

Merged via the queue into apache:main with commit 30eccb4 Aug 4, 2026
38 checks passed
@alamb
alamb deleted the alamb/update_pr_template branch August 4, 2026 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

development-process Related to development process of DataFusion documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants