Hi all, FWIW, I'm currently reviewing PR [2887], and despite the new template, the contributor omitted license headers, the changelog entry, and needed additional guidance on testing. The template proved largely ineffective.
I've created a much shorter, alternative PR template: [2945]. It's only 13 lines, a significant reduction from the current 65, which I believe keeps visual clutter to an acceptable minimum. Would that one be an acceptable compromise? Thanks, Alex [2887]: https://github.com/apache/polaris/pull/2887 [2945]: https://github.com/apache/polaris/pull/2945 On Wed, Oct 29, 2025 at 10:08 PM Yufei Gu <[email protected]> wrote: > > Rework is part of the software engineering. Better rework early than > wasting any reviewers' time. > > Yufei > > > On Wed, Oct 29, 2025 at 12:37 PM Dmitri Bourlatchkov <[email protected]> > wrote: > > > 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. > > > > > > > > >
