Update PR template

[madelondohmen]

This ticket is part of #2886.

Is your feature request related to a problem? Please describe.
The PR template we're currently using is sometimes lacking information which is relevant for the reviewers/QA'ers.

Describe the solution you'd like
Update te current PR template.

To think about
We are also designing a new job request template and a bug report template. The template for PRs repeats some of the same sections. We need to think about how to minimize the effort for the creator of the PR, while making the PR as clear as possible for the reviewer.

The sections cannot be omitted from the PR template (with only a reference to the problem) because a PR is sometimes created separately from a ticket. One solution may be to copy-paste it from the ticket.

[madelondohmen]

Idea for a PR template

---

Type of changes

What types of changes does your code introduce to OpenKAT? Put an 'x' in the boxes that apply.

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Documentation update
• Other, namely:

About this PR

Description

Why have you created this PR?
If this PR is related to a problem, what problem does it solve? Under what circumstances is this problem occurring?

Bugfix

In case of a bugfix, please fill in the following details. If not, please go to “Solutions” and continue there.

Expected behavior
Tell us the behavior that you expected.

Actual behavior
Tell us what happened instead.

To reproduce
Steps to reproduce the behavior:

1. Go to '...'
2. Click on '....'
3. Scroll down to '....'
4. See error

Screenshots and logs of the problem
What do you see when the problem is triggered? If possible add logs, screenshots, videos or other parts of documentation that show the problem.

Solutions

Possible solutions

Describe possible ways to solve the problem. What other options have you considered? Why would those options (not) work?

Chosen solution

Which solution have you chosen?

Changes
What changes are you making with this PR? Why are those necessary?

Demo of your solution

Screenshot or video of your working solution.

Issues

Closes

List of tickets that are closed with your solution.
Closes #

Opens

List of follow up tickets relating to your problem. For example tech depth tickets.

[stephanie0x00]

I'd love a QA section with 'QA notes` such as #2709

[madelondohmen]

Ofcourse! How could I forget the whole checklist section... Whoops. The checks might need an update as well. Does anyone have suggestions?

New version:

---

Type of changes

What types of changes does your code introduce to OpenKAT? Put an 'x' in the boxes that apply.

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Documentation update
• Other, namely:

About this PR

Description

Why have you created this PR?
If this PR is related to a problem, what problem does it solve? Under what circumstances is this problem occurring?

Bugfix

In case of a bugfix, please fill in the following details. If not, please go to “Solutions” and continue there.

Expected behavior
Tell us the behavior that you expected.

Actual behavior
Tell us what happened instead.

To reproduce
Steps to reproduce the behavior:

1. Go to '...'
2. Click on '....'
3. Scroll down to '....'
4. See error

Screenshots and logs of the problem
What do you see when the problem is triggered? If possible add logs, screenshots, videos or other parts of documentation that show the problem.

Solutions

Possible solutions

Describe possible ways to solve the problem. What other options have you considered? Why would those options (not) work?

Chosen solution

Which solution have you chosen?

Changes
What changes are you making with this PR? Why are those necessary?

Demo of your solution

Screenshot or video of your working solution.

Issues

Closes

List of tickets that are closed with your solution.
Closes #

Opens

List of follow up tickets relating to your problem. For example tech depth tickets.

---

Checklist for the developer

Code Checklist

• All the commits in this PR are properly PGP-signed and verified.
• This PR only contains functionality relevant to the issue; tickets have been created for newly discovered issues.
• I have written unit tests for the changes or fixes I made.
• For any non-trivial functionality, I have added integration and/or end-to-end tests.
• I have performed a self-review of my code and refactored it to the best of my abilities.

Communication

• I have informed others of any required .env changes files if required and changed the .env-dist accordingly.
• I have made corresponding changes to the documentation, if necessary.
• I have included comments in the code to elaborate on what is not self-evident from the code itself, including references to issues and discussions online, or implicit behavior of an interface.

---

Checklist for code reviewers:

Copy-paste the checklist from the docs/source/templates folder into your comment.

---

Checklist for QA:

Copy-paste the checklist from the docs/source/templates folder into your comment.

[ammar92]

Imho this template is too big for a PR. I believe most of this should already be handled by the corresponding issue.

Type of changes

What types of changes does your code introduce to OpenKAT? Put an 'x' in the boxes that apply.

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Documentation update
• Other, namely:

This section is redundant, GitHub has introduced labels for these. Branch names should also be a good indication for the type of PR (although there isn't a written branch name convention yet).

About this PR

Description

Why have you created this PR? If this PR is related to a problem, what problem does it solve? Under what circumstances is this problem occurring?

This should already be covered by the issue (for both feature requests and bugs) the PR is referencing to.

Bugfix

In case of a bugfix, please fill in the following details. If not, please go to “Solutions” and continue there.

There's a bug fix section only, while earlier you mention also different types of changes, which makes this section inconsistent. Also everything in this section should already be included in the bug issue template (except for the logs, which could be added next to screenshots)

Solutions

In the current PR template, this should already be included in the "changes" section.

Possible solutions

Describe possible ways to solve the problem. What other options have you considered? Why would those options (not) work?

Should be already mentioned in the feature request template.

Opens

List of follow up tickets relating to your problem. For example tech depth tickets.

GitHub references this PR automatically in the timeline if this PR gets mentioned in another ticket. Although it doesn't really hurt to still have this section.

[madelondohmen]

I get your point, most of this should already be covered in the issue. But what if a PR is created without an issue? I know it isn't supposed to happen, but it does happen sometimes.

That's why @stephanie0x00 created the first version of the new PR template (from which I copied most of these sections). She said that sometimes they were missing context. What would be a good solution here? To never allow a PR without an issue anymore? What do you guys think?

And @ammar92, what DO you want to see in a PR template? What is useful to you?

[ammar92]

I get your point, most of this should already be covered in the issue. But what if a PR is created without an issue? I know it isn't supposed to happen, but it does happen sometimes.

Thanks for your input. That's a good point indeed. While we generally prefer to have an issue for every PR, we do allow some exceptions, such as package updates or PRs coming from contributors. Imho the current PR template should be sufficient, and in cases where the context is unclear, this should lead to questions and discussion.

That's why @stephanie0x00 created the first version of the new PR template (from which I copied most of these sections). She said that sometimes they were missing context. What would be a good solution here? To never allow a PR without an issue anymore? What do you guys think?

See my point above.

And @ammar92, what DO you want to see in a PR template? What is useful to you?

I want to see good PRs and issues, regardless of any template. This requires a fair amount of effort by the author to communicate effectively. In my experience having more questions, form inputs and other means of questionnaire does improve the consistency, but it doesn't necessarily improve the quality. We still see unnecessary boilerplate (e.g. Closes ... not being removed by the author, even though it's minimal effort; and questions left unanswered) hanging around or unchecked checkmarks. Most importantly, there's often unclear context. Although the checklist can be improved; because the current checklist might be ambiguous, since some of the marks aren't always clear whether they should be checked or not.

[madelondohmen]

Alright, you've got some good points there! If the current PR template doesn't cause any problems for you, I think it might not be necessary to change it. (But we can think about updating the checkbox.)

But ofcourse it depends on what the other reviewers think.

[madelondohmen]

During the discussion meeting, we've concluded to leave the PR template the way it is and just add two things:

• Add a line with something like "Create an issue or write an extended PR".
• Add a section for Stephanie with "QA notes" containing comments/extra explanation for QA to test properly.

It would then look something like this:

---

Changes

Please describe the essence of this PR in a few sentences. Mention any breaking changes or required configuration steps.

Issue link

You have to create an issue to link to this PR. If this really is not possible, write a very detailed description here and add this PR to the project board directly..

Please add the link to the issue after "Closes".

Closes ...

Demo

Please add some proof in the form of screenshots or screen recordings to show (off) new functionality, if there are interesting new features for end-users.

QA notes

Please add some information for QA on how to test the newly created code.

---

Code Checklist

• All the commits in this PR are properly PGP-signed and verified.
• This PR only contains functionality relevant to the issue; tickets have been created for newly discovered issues.
• I have written unit tests for the changes or fixes I made.
• For any non-trivial functionality, I have added integration and/or end-to-end tests.
• I have performed a self-review of my code and refactored it to the best of my abilities.

Communication

• I have informed others of any required .env changes files if required and changed the .env-dist accordingly.
• I have made corresponding changes to the documentation, if necessary.
• I have included comments in the code to elaborate on what is not self-evident from the code itself, including references to issues and discussions online, or implicit behavior of an interface.

---

Checklist for code reviewers:

Copy-paste the checklist from the docs/source/templates folder into your comment.

---

Checklist for QA:

Copy-paste the checklist from the docs/source/templates folder into your comment.

[madelondohmen]

Besided this, we have to think about the "Code checklist" section. It's a bit outdated. Any ideas for useful checks?

[madelondohmen]

Updated the code checklist in the PR. (Approved by @ammar92 and @stephanie0x00)

As discussed before, people who do the review have to make sure all checkboxes are checked. If not, a ticket cannot be merged. The reviewers have to check this and tell the developers if something is missing.

---

Changes

Please describe the essence of this PR in a few sentences. Mention any breaking changes or required configuration steps.

Issue link

You have to create an issue to link to this PR. If this really is not possible, write a very detailed description here and add this PR to the project board directly..

Please add the link to the issue after "Closes".

Closes ...

Demo

Please add some proof in the form of screenshots or screen recordings to show (off) new functionality, if there are interesting new features for end-users.

QA notes

Please add some information for QA on how to test the newly created code.

---

Code Checklist

• All the commits in this PR are properly PGP-signed and verified.
• This PR only contains functionality relevant to the issue.
• I have written unit tests for the changes or fixes I made.
• I have checked the documentation and made changes where necessary.
• I have performed a self-review of my code and refactored it to the best of my abilities.

• Tickets have been created for newly discovered issues.
• For any non-trivial functionality, I have added integration and/or end-to-end tests.
• I have informed others of any required .env changes files if required and changed the .env-dist accordingly.
• I have included comments in the code to elaborate on what is not self-evident from the code itself, including references to issues and discussions online, or implicit behavior of an interface.

---

Checklist for code reviewers:

Copy-paste the checklist from the docs/source/templates folder into your comment.

---

Checklist for QA:

Copy-paste the checklist from the docs/source/templates folder into your comment.