From 11e679147d35f460ecec40052d33f5e6b5d3cac2 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sat, 30 Aug 2025 19:46:47 +0200 Subject: [PATCH 1/3] CONTRIBUTING: use pr-template short link This is explicitly defined below the header, but wasn't used. --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f3dfae0563b3..8cc95f7617c1 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -91,7 +91,7 @@ This section describes how changes can be proposed with a pull request (PR). 6. [Create a pull request](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request#creating-the-pull-request) from the new branch in your Nixpkgs fork to the upstream Nixpkgs repository. Use the branch from step 1 as the PR's base branch. - Go through the [pull request template](#pull-request-template). + Go through the [pull request template][pr-template]. 7. Respond to review comments and potentially to CI failures and merge conflicts by updating the PR. Always keep it in a mergeable state. From 8fa15577842ba38283385ed0ba5b9fabb87e5879 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sat, 30 Aug 2025 19:49:13 +0200 Subject: [PATCH 2/3] CONTRIBUTING: clarify pushing to others' PRs is allowed While squash-merges have been discouraged from already, they become impossible to do when the Merge Queue is enabled. In certain scenarios it was tolerated to use these, however, thus we introduce an acceptable replacement instead. By making explicit that pushing to contributors PRs is allowed by default, it should be easier for committers to do so. --- CONTRIBUTING.md | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8cc95f7617c1..e5f54f44c503 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -526,8 +526,15 @@ While this potentially can be understood by reading code, PR discussion or upstr Simple package version updates need to include the attribute name, old and new versions, as well as a reference to the release notes or changelog. Package upgrades with more extensive changes require more verbose commit messages. -Pull requests should not be squash-merged, as this discards information including detail from commit messages, GPG signatures, and authorship. -Many pull requests don't make sense as a single commit anyway. +## Merge conventions + +To make changes on commit structure and commit messages or apply simple suggestions, committers are encouraged to [checkout the PR](https://cli.github.com/manual/gh_pr_checkout) and push directly to the contributor's branch before merging. +Committers will carefully weigh the cost of another review cycle against the feelings of the contributor when pushing to their branch. +They should also transparently communicate which changes they made. +If a contributor does not want committers to push to their branch, they must uncheck the "Allow edits and access to secrets by maintainers" box explicitly. + +> [!WARNING] +> Committers: Branches created via `gh pr checkout` can't be pushed with `--force-with-lease`, so do a sanity check before pushing. ## Code conventions [code-conventions]: #code-conventions From 0ba05751f984c92083b936c2dc321be76c9b80e3 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sat, 30 Aug 2025 19:54:42 +0200 Subject: [PATCH 3/3] CONTRIBUTING: default to non-blocking comments By making blocking comments explicit, it will be easier to discard certain nits. --- CONTRIBUTING.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e5f54f44c503..7530df9d7c2c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -526,7 +526,15 @@ While this potentially can be understood by reading code, PR discussion or upstr Simple package version updates need to include the attribute name, old and new versions, as well as a reference to the release notes or changelog. Package upgrades with more extensive changes require more verbose commit messages. -## Merge conventions +## Review and Merge conventions + +Comments on Pull Requests are considered non-blocking by default. +Every blocking comment must be explicitly marked as such by using GitHub's "Request Changes" review type. +A reviewer who submits a blocking review should be available for discussion and re-review. +An abandoned review may be dismissed after reasonable time was given at the discretion of the merger. + +All suggestions for change, blocking or not, should be acknowledged before merge. +This can happen implicitly by applying the suggestion, or explicitly by rejecting it. To make changes on commit structure and commit messages or apply simple suggestions, committers are encouraged to [checkout the PR](https://cli.github.com/manual/gh_pr_checkout) and push directly to the contributor's branch before merging. Committers will carefully weigh the cost of another review cycle against the feelings of the contributor when pushing to their branch.