docs(): Add pull request approval policy and update references#31
docs(): Add pull request approval policy and update references#31pon-james wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a new, risk-based Pull Request Approval Policy document and updates existing contributor documentation to point to it, so contributors can more clearly determine when peer approval is required before merging.
Changes:
- Added a new
pull-request-approval-policy.mddefining approval levels, response windows, and expectations for authors/reviewers (including AI-assisted work). - Updated
pull-requests.mdto link to the new approval policy. - Updated
README.mdto link to the new approval policy.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pull-requests.md | Adds an early reference to the new approval policy from the existing PR best-practices doc. |
| pull-request-approval-policy.md | Introduces the new risk-based approval policy and guidance. |
| README.md | Adds a contributor-facing pointer to the new approval policy. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
|
||
| ## Approval policy | ||
|
|
||
| Use the highest applicable level below. If a PR contains several kinds of change, apply the strictest approval rule. |
There was a problem hiding this comment.
Should we explicitly mention library updates here?
There was a problem hiding this comment.
Major , Minor dependency updates must be reviewed. Patches can be auto approved with caution
|
|
||
| ## Response window for low-risk changes | ||
|
|
||
| For **Level 1** changes, the default response window is **4 business hours** during the team's normal working day. |
There was a problem hiding this comment.
; so creat PR before noon, get an answer before end of the working day
There was a problem hiding this comment.
Todo: add an example to make it more explicit
|
|
||
| ## What cannot be self-merged | ||
|
|
||
| The following always require peer approval, even if the code change looks small: |
There was a problem hiding this comment.
Seems a bit of a duplicate compared to the previous section
| ## Expectations for authors | ||
|
|
||
| Before merging any pull request, the author should be able to say yes to all of the following: | ||
|
|
There was a problem hiding this comment.
- I reserved time in my agenda to merge ?
There was a problem hiding this comment.
That the author is prepared to follow up when the review is followed up on in the required time frame.
| Reviewers help the team maintain both quality and throughput. | ||
|
|
||
| * If you can review, respond promptly. | ||
| * If you cannot review in a reasonable time, decline quickly so the author can ask someone else. |
There was a problem hiding this comment.
If you know you cant review decline immediately ?
Most reviewers can directlty check their agenda and check for an opening
There was a problem hiding this comment.
The reviewer must explicitly decline a review request. Otherwise you implicitly accept the invitation.
|
|
||
| * If you can review, respond promptly. | ||
| * If you cannot review in a reasonable time, decline quickly so the author can ask someone else. | ||
| * Block only for issues that matter: correctness, security, maintainability, operability, or clarity of intent. |
|
|
||
| ## Emergency changes | ||
|
|
||
| Production incidents sometimes require immediate action. In those cases, follow the incident process defined by the team or platform owner. |
There was a problem hiding this comment.
But then reviewer should also drop everything?
|
General comment to @pon-james @zeger-pon, do we also want to distinguish projects based on priority/importance? A review for a feature change for De Onderdelendienst might have a different priority than the same kind of review for the Gen AI platform or Pulse. Can I get your opinion on this? |
This pull request introduces a new, detailed Pull Request Approval Policy and updates documentation to clarify when peer approval is required before merging changes. The main goal is to make the approval process clearer, risk-based, and more efficient, reducing unnecessary waiting for low-risk changes while maintaining quality and accountability.
Documentation updates:
Added a new
pull-request-approval-policy.mdfile that outlines a risk-based approval process, specifying when contributors may self-merge and when peer review is mandatory. The policy includes examples, expectations for authors and reviewers, guidance for AI-assisted development, and recommendations for repository settings.Updated
README.mdto reference the new approval policy, providing contributors with a direct link for guidance on approval requirements.Updated
pull-requests.mdto reference the new approval policy, ensuring contributors know when approval is required and when low-risk changes may proceed without waiting.