Skip to content

Conversation

@jeremy379
Copy link
Member

New:

Description

  • Remove the checklist section as it is not used, and worst, often let it remain untouched, and included in the merged commit.

Old

Description

Checklist

Code quality

  • I have performed a self-review of my on code.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added tests to cover edge cases.
  • All new and existing tests passed.

Documentation

  • I have updated the relevant documentation where needed.

Deployment

  • My changes require a migration or other data-related update.
    • I have applied the necessary migration or data updates.
  • I have updated the deployment instructions if necessary.

@elimentz
Copy link

elimentz commented Sep 9, 2025

I quite like this template. I agree that it's underused but I would argue we should put more effort into it rather than throw it away

@MarvinKilsdonk
Copy link

I don't like the template as it is now. Code quality its part of your daily job right? I also not have a checklist that i came out of bed everyday, and take some food and drink some water.

The part for the exceptions like update the release document and if you put the migrations (because they are on a different repository) its can be handy if its more mandatory. Not sure if this is possible but if you put in that you need migrations you have to add the PR for this migrations. And this becomes more of the review part. That the reviewer also reviews the migrations or the notes in the release document.

@jeremy379
Copy link
Member Author

jeremy379 commented Oct 10, 2025

I made a check, to know how the checklist is used.
Seems in the last 3 month, only 16,6% of the PR used it

Total PRs analyzed: 193

Template unchanged : 59 (30.6%)
Description filled, no checklist checked: 45 (23.3%)
Checklist removed, description kept : 50 (25.9%)
Other — checklist used : 32 (16.6%)
Other : 7 (3.6%)

I don't think, wathever we do, that we could have that checklist filled by developpers if it's not enforced by a ci (like checking title)

Aside of that, I don't think a checklist is really relevant, I'd prefer having meaningful information

Here is a suggestion of template which will provide information on the PR, if there is a risk and information about releasing.
The verification part is the checklist and can be much shorte (knowing we already have a lot of CI running)

Description

  • Ticket:
  • Type:

Impact & Rollout

  • Risk:
  • Rollout:
  • Monitoring/ Alerts:
  • Rollback plan:

Verification

  • Critical path manually verified
  • Tests updated where it matters
Notes (optional)

Context

Data / Migrations

Security & Privacy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants