Hi Yufei, Prashant, and All,
I fully support your intention of guiding contributors toward high quality
commits.
However, I do not think the current PR template helps us achieve this goal.
For example, in [2936] the author's message is quite adequate, but template
headers appear to be an obstacle.
From a more general perspective, here's why I think the template is not
effective:
* Contributors normally start from the source code and docs, which have
contribution guidelines.
* Contributors work out a code change and commit it _locally_ first.
* Contributors push the code change to GH and open a PR.
* At this point the PR template kicks in, but the commit message is already
made and is immutable (all commits are immutable in git).
* The author has to either ignore the template (natural reaction) or rework
the message to fit the template.
* When the PR is merged, IIRC, the committer will have to rework the final
merge message, because GH makes the default one from original commit
messages. not from the PR description.
All the points that the PR template attempts to enforce can be covered in
the contribution guide (which is normally reviewed upfront) with more
efficiency, I believe.
If the author does not follow the template (which is common) and reviewers
start enforcing the template by means of review comments, we might as well
do that without having the PR template by referring to the contribution
guidelines.
As for checkboxes in the PR template (as suggested by François), that might
work as a reminder. The PR author will not have to edit that part of the
description in text and the checkboxes do not need to be in the final merge
message. My personal opinion is that this kind of reminder is preferable to
the section headers in the current PR template, but it is also too late
(per workflow points above).
WDYT?
[2936] https://github.com/apache/polaris/pull/2936
Thanks,
Dmitri.
On Mon, Oct 27, 2025 at 12:39 PM Yufei Gu <[email protected]> wrote:
+1 on adding necessary templates, they help guide contributors toward
higher-quality commits. Templates benefit both new and experienced
contributors.
Here are a few examples I find especially useful:
1. Why are these changes needed? What’s the use case?
2. How was this patch tested?
3. Checkbox: I have commented my code, particularly in hard-to-understand
areas
Do these apply to all PRs? Not necessarily. They’re meant as reminders,
not
constraints.
How effective are they? It varies, some contributors may ignore them
entirely, but they still add value by setting expectations and
encouraging
better documentation.
Yufei
On Sun, Oct 26, 2025 at 9:50 AM Francois Papon <[email protected]>
wrote:
Hi,
Just keep in mind that the purpose of these tempates is to help to
growth the community of the project and having new committers, so it
can
help new comer to learn how the ASF is working, even if reading the
community guide lines should be the starter.
regards,
François
Le 26/10/2025 à 12:02, Alexandre Dutra a écrit :
Hi all,
I disagree with the idea that commit messages should be devoid of
informative content. On the contrary, detailed commit messages are
crucial for understanding the purpose of changes, especially large
ones, when browsing the commit log. I'm not advocating for
excessively
long messages, but a reasonable amount of text can be highly
beneficial. I personally always provide detailed information in my
commits, a practice even endorsed by the author of Git himself [1].
However, this deviates from our main discussion regarding PR
templates.
The argument that we should adopt long-ish templates because projects
X, Y, or Z do so is unconvincing. We cannot assume the majority is
always correct, and we don't know if the templates are causing
frustration and deterring first-time contributors from opening PRs.
Furthermore, our existing contribution guidelines, **which
contributors are expected to read**, already contain extensive
instructions for PR contributors [2]:
The Pull Request description should provide the background
(rationale)
of the change and describe the changes in way [sic] that someone who
has
no
prior knowledge can understand the rationale of the change and the
change
itself. [...] Test that your changes work by adapting or adding tests.
Verify the build passes.
To me, this is more than enough. I tend to disregard checklists and
TODO lists as a reviewer, as there's no guarantee contributors have
completed them in good faith (and besides, the Definition of Done is
a
subjective notion).
On the changelog topic: I agree that maintainers should update it, if
the PR author didn't, in order to accelerate the review process. This
kind of polishing can be more easily achieved when PR authors allow
edits by maintainers, as a maintainer can push a "polishing commit"
to
the PR before squashing. Spring Boot does this often to enforce
formatting, naming conventions, changelog entries and other similar
chores that PR authors aren't familiar with. I think we could do the
same in Polaris.
Finally, I also prefer when the PR template is included in an HTML
comment. It greatly reduces the visual clutter when the PR
description
is rendered. Two good examples of concise-ish templates using HTML
comments are: Spring Boot [3] and Quarkus [4].
To move forward on this topic, I propose a compromise: let's
implement
a template, but keep it as concise as possible, linking to the
Polaris
contribution guidelines. If we need to add more instructions, let's
enrich the guidelines themselves, not the PR template.
Would this approach be acceptable to everyone?
Thanks,
Alex
[1]:
https://github.com/torvalds/subsurface-for-dirk/blob/master/README.md?plain=1#L128-L157
[2]:
https://github.com/apache/polaris/blob/main/CONTRIBUTING.md#opening-a-pull-request
[3]:
https://github.com/spring-projects/spring-boot/blob/main/.github/PULL_REQUEST_TEMPLATE.md?plain=1
[4]:
https://github.com/quarkusio/quarkus/blob/main/.github/PULL_REQUEST_TEMPLATE.md?plain=1
On Sun, Oct 26, 2025 at 8:23 AM Jean-Baptiste Onofré <
[email protected]>
wrote:
Hi
I do agree that the commit message should be short and concise (it’s
what I
said in my previous message). It should not contain anything about
the
issue details or use case: that’s in the github issue.
The PR template should just reflect the contributing guideline
helping
the
contributor.
Several projects were very demanding when a contributor creates a PR
and
they now use a lighter approach because it had an impact to
contributions.
Regards
JB
Le sam. 25 oct. 2025 à 23:58, Eric Maynard <
[email protected]>
a
écrit :
I strongly disagree with the notion that every single commit
message
should
contain information explaining the entire PR’s intent. I have
scarcely
seen
a project or organization where that was so and my sense is that
this
would
be unnecessarily burdensome for contributors.
I do however think the PR template could use some tuning. The fact
is
that
it’s not often followed, and so each PR tends to describe itself in
its own
way which can contribute to the mental load it takes to review a
string of
PRs. It might be better to adjust the template and more rigorously
enforce
its use. The best template, after all, is one that gets used :)
—EM
On Sat, Oct 25, 2025 at 9:38 AM Francois Papon <[email protected]>
wrote:
Hi,
I think it's better to have a small PR template to guide users to
contribute and also to help the reviewer.
Having some checkbox can be useful, there is an example in some
ASF
projects
(
https://github.com/apache/shiro/blob/main/.github/pull_request_template.md
)
Agreed with JB, the changelog should not be updated by the
contributor.
regards,
François
Le 25/10/2025 à 07:18, Jean-Baptiste Onofré a écrit :
Hi Dmitri
Thanks for starting (re-starting :)) the discussion.
Generally speaking, a template is useful if it guides and informs
the
contributor. Personally, most of the time, I don't think the
templates
are super helpful (due to the content).
If the template has to be informative for the contributors, it
should
not ask more than what the contributor can easily do. I think we
should avoid too much constraints that can be a brake to
contribution.
Specifically about our current template, some thoughts:
* If the PR is "linked" to an issue, there's no need to describe
the
changes in the PR because it should be in an issue, and so in the
commit message. So, maybe we should encourage people to
use/create
corresponding issues. If there's no issue, then it's OK to
describe
quickly the PR purpose. We should phrase that better than "What
changes were proposed in this pull request?".
* About the test, a change should have corresponding tests if
needed.
Having a manual test scenario is acceptable and described in the
PR.
I
would propose to have just a checkbox like "do you have
corresponding
tests", if not the user can provide a manual test case.
* About the CHANGELOG, imho; it's not something we should ask the
contributor, because he doesn't really know the changelog details
and
format needed (potentially). So, I would say it's more something
that
the reviewer should help with, eventually guiding the contributor
in
terms of changelog.
As today nothing is "enforced" about the template, it's fine. If
we
can improve the contributor experience, it's even better :)
Regards
JB
On Fri, Oct 24, 2025 at 4:24 PM Dmitri Bourlatchkov <
[email protected]>
wrote:
Hi All,
I'd like to (re-)open a discussion for our PR template.
Using [2883] and other recent PRs as an example of the
_description_
usage
(not code).
* What changes were proposed in this pull request?
* Why are the changes needed?
I do not find these sections useful. Any responsible PR author
should
cover
these items in every commit message anyway. Having special
sections
is
more
of a nuisance IMHO as it require working with test in GH UI to
fill
them
out or delete (while the idea is already in the commit message).
* Does this PR introduce any user-facing change?
This is a very broad topic and probably depends a lot on
specific
use
cases. In principle, any code change is a user-facing change as
it
affects
how Polaris works.
* How was this patch tested?
Again, responsible PR authors should include CI tests as
appropriate.
Reviewers should hold contributors accountable for that. Having
to
fill
this section out is overhead, IMHO.
* CHANGELOG.md
This section header is not informative as it stands.
In any case as discussed before [2] reviewers have a duty to
requests
changelog changes if they would be meaningful, but got missed.
While
PR
authors are encouraged to add CHANGELOG entries proactively, I
do
not
think
having a forced sub-section for that in each PR is worth the
extra
complications in the PR submission workflow.
I propose:
* Remove all forced section headers from the PR template (we
could
keep
them in comments). Basically use an empty default template.
* Add changelog notes to the Contributing Guidelines on the site
[1]
[1]
https://polaris.apache.org/community/contributing-guidelines/
[2]
https://lists.apache.org/thread/c9y0f0z7nyoclvtzr12v8ryqq55dqzd5
[2883] https://github.com/apache/polaris/pull/2883
WDYT?
Thanks,
Dmitri.
