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. > > >
