Thank you for opening this discussion! IMHO having a template is super useful. Here is my reasoning, and I'd love to know what you all think.
PR templates are a common and effective practice, as several major Apache projects already use them with very similar questionnaires: - *Apache Spark:* PULL_REQUEST_TEMPLATE <https://github.com/apache/spark/blob/master/.github/PULL_REQUEST_TEMPLATE> - *Apache Parquet:* PULL_REQUEST_TEMPLATE <https://github.com/apache/parquet-java/blob/master/.github/PULL_REQUEST_TEMPLATE.md> I also want to share my perspective on some of the points that were shared earlier: > Any responsible PR author should cover these items in every commit message anyway. While we have many responsible authors, we also have new contributors. A template is invaluable for guiding them toward a high-quality submission. It also helps reviewers understand the context and provide better feedback. I don't want to pinpoint specific examples, but there are plenty of PRs with literally zero lines of description. This makes it extremely difficult for reviewers to understand the author's goals or even participate in a productive discussion. > In principle, any code change is a user-facing change as it affects how Polaris works. I agree with this in principle, but there are exceptions like refactoring that don't introduce user-facing changes. However, the *reason* for the refactor is critical. Is it a matter of style? Is it to enable a new feature that the current structure prevents? Or is it a maintenance improvement, like boosting performance? A template prompts the author to provide this crucial context. > How was this patch tested? This section is very important, IMHO. For example, we have dormant tests (like our cloud-provider tests) that don't run as part of the standard CI pipeline. If a patch introduces a behavior change, we need to ensure those regression tests are run. Likewise, if a change affects something specific like S3 status code handling, it requires more than just unit tests, sometimes manual verification too. A template serves as a reminder to add sufficient test coverage for both positive and negative cases wherever possible and if some manual testing is done provide some context to the reviewers > CHANGELOG.md If we want CHANGELOG.md to be the source of truth for our release notes, it would be highly beneficial to prompt authors and reviewers to update it with every PR. Otherwise, it becomes an enormous task for the release manager to keep it up-to-date. I don't believe this is something we can fully automate. ------------------------------ Finally, even with a template, I have seen PRs with no description still being raised and merged. Ultimately, if the author and reviewer have a mutual understanding and are okay with it, I think we should be fine. Best, Prashant Singh On Fri, Oct 24, 2025 at 7:25 AM 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. >
