Define a pull request template

[benoit74]

Some projects have a pull_request_template.yml, but it does not look like there is a convention on this yet.

The template I've found so far is:

## Rationale

[//]: # (Briefly explain the reason behind this change.)

<!--
Issue: [Title](link) or #123 for Github issues.
-->

## Changes

[//]: # (Summarize what has changed.)

Some inconvenient I find in this template:

• I do not find the term "Rationale" very obvious
• There is no mention about the fact that we must add "Fix: ###" to automatically close issues.
• The "Issue" template is misleading, it looks like it will do something automatically while it doesn't + all issues should be Github issues (or most at least), so no need to propose something for the case where it is not

ChatGPT suggested me the following sections:

## Description

<!-- Describe the purpose of this pull request -->

## Changes Made

<!-- List the changes made in this pull request -->

## Screenshots (if applicable)

<!-- Include screenshots or images demonstrating the changes -->

## Testing

<!-- Explain how the changes were tested -->

## Related Issues

<!-- Mention any related issues or link to them -->

## Checklist

- [ ] Code follows project standards
- [ ] Tests have been added/updated
- [ ] Documentation has been updated
- [ ] Changelog entry added

And it should be named pull_request_template.md (it's not a yaml file 😅)

My remarks on ChatGPT suggestion:

• I think the related issues should be clearer about the fact that we expect one or more "Fix: ###" and I would like to put this at the beginning of the PR, just after the description (or even before that) so that it is more obvious why we are proposing this PR
• in the checklist:
  • we should add a link to our project standards so that it is obvious
  • most scrappers do not have tests (yet, at least) ; and usually there structure makes testing a daunting task ; I think we should not add this check in most scrapers yet
  • documentation should refer to usage + wiki

I welcome all suggestions on this, I can handle the creation of a PR once arguments have settled a little bit.

[rgaudin]

I agree with your suggestions. To me there are only two objectives:

• properly linking to one or multiple existing issues. This is important to acknowledge a common requirement of the PR goal. Comment containing sample Fixes #xx seems 👍
• Getting a proper description of what this is about and what it does.

I don't care for structure ; most of the time it's a bad idea.

A single free text with good directions seems better to me. I really like the idea of providing links to our guidelines and what we expect (instead of just an obscure Rationale heading) and I think the checklist in the end is a great to to avoid structure while making sure people are informed about what's expected.

We can also specify that we are being notified automatically and that we'll assign it to a reviewer within a couple days.

[benoit74]

👍

We can also specify that we are being notified automatically and that we'll assign it to a reviewer within a couple days.

I agree that first time committers should be informed that we will take care of their PR and there is no need to worry / notify us on many channels like we encounter too often. But at the same time, I would like they get the habit that we also have to ask for review on their own, allowing us to be sure when work is ready to be reviewed. We should find a formulation which makes this clear. WDYT?

[rgaudin]

I agree with the idea but I'm not sure how practical that is:

• Usually external contributor don't have the possibility to request a review. I see that on other repos I contribute to. Maybe that's something that can be configured in GH.
• Should they be able, they wouldn't know who to ask review from. GH makes suggestions but it's based on commit-history for touched files and this frequently summons old-contributors who are long gone.
• PRs are not supposed to trigger the tests on their own (when there are!) so we can't ask them to wait for that
• Opening a PR in Draft still notifies people watching the repo
• Turning a WIP PR into a Ready one doesn't notify people subscribed to the repo or the PR

And I think that's the point of making a pull-request: you're supposed to be ready when you do (for initial review).

Maybe we can explain that a reviewer will authorize the tests (if that applies) and should those not pass, it's their responsibility to fix it and add a comment in the PR. Or we plain request that they mention when it's ready to review.