Skip to content

Commit 515b174

Browse files
wolfgangwaltherwolfgangwalther
committed
workflows/check-cherry-picks: post review comments
Instead of failing the job, the workflow will now post review comments as "Request Changes". This makes the feedback more readily visible and avoids having to merge despite a failing CI job. It is also a pre-requisite to enable required status checks / required workflows in the future. Committers are asked to confirm the differences by explicitly dismissing the generated review. After dismissal, the related review comment will automatically be marked as "resolved". The comments only report warnings and errors. Reviews are automatically dismissed when they have been addressed by the author and no problems remain. If problems remain, existing, still pending, review comments will be updated. If the same problems had already been dismissed earlier, no new review comment will be created either.
1 parent 3dff9c3 commit 515b174

File tree

4 files changed

+165
-11
lines changed

4 files changed

+165
-11
lines changed

‎.github/workflows/check-cherry-picks.yml‎

Lines changed: 100 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@ on:
1010
- 'staging-**'
1111
- '!staging-next'
1212

13-
permissions: {}
13+
permissions:
14+
pull-requests: write
1415

1516
jobs:
1617
check:
@@ -24,8 +25,105 @@ jobs:
2425
path: trusted
2526

2627
- name: Check cherry-picks
28+
id: check
29+
continue-on-error: true
2730
env:
2831
BASE_SHA: ${{ github.event.pull_request.base.sha }}
2932
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
3033
run: |
31-
./trusted/ci/check-cherry-picks.sh "$BASE_SHA" "$HEAD_SHA"
34+
./trusted/ci/check-cherry-picks.sh "$BASE_SHA" "$HEAD_SHA" checked-cherry-picks.md
35+
36+
- name: Prepare review
37+
if: steps.check.outcome == 'failure'
38+
uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1
39+
with:
40+
script: |
41+
const { readFile, writeFile } = require('node:fs/promises')
42+
43+
const job_url = (await github.rest.actions.listJobsForWorkflowRun({
44+
owner: context.repo.owner,
45+
repo: context.repo.repo,
46+
run_id: context.runId
47+
})).data.jobs[0].html_url + '?pr=' + context.payload.pull_request.number
48+
49+
const header = await readFile('trusted/ci/check-cherry-picks.md')
50+
const body = await readFile('checked-cherry-picks.md')
51+
const footer =
52+
`\n_Hint: The diffs are also available in the [runner logs](${job_url}) with slightly better highlighting._`
53+
54+
const review = header + body + footer
55+
await writeFile('review.md', review)
56+
core.summary.addRaw(review)
57+
core.summary.write()
58+
59+
- name: Request changes
60+
if: ${{ github.event_name == 'pull_request_target' && steps.check.outcome == 'failure' }}
61+
uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1
62+
with:
63+
script: |
64+
const { readFile } = require('node:fs/promises')
65+
const body = await readFile('review.md', 'utf-8')
66+
67+
const pendingReview = (await github.paginate(github.rest.pulls.listReviews, {
68+
owner: context.repo.owner,
69+
repo: context.repo.repo,
70+
pull_number: context.payload.pull_request.number
71+
})).find(review =>
72+
review.user.login == 'github-actions[bot]' && (
73+
// If a review is still pending, we can just update this instead
74+
// of posting a new one.
75+
review.state == 'CHANGES_REQUESTED' ||
76+
// No need to post a new review, if an older one with the exact
77+
// same content had already been dismissed.
78+
review.body == body
79+
)
80+
)
81+
82+
if (pendingReview) {
83+
await github.rest.pulls.updateReview({
84+
owner: context.repo.owner,
85+
repo: context.repo.repo,
86+
pull_number: context.payload.pull_request.number,
87+
review_id: pendingReview.id,
88+
body
89+
})
90+
} else {
91+
await github.rest.pulls.createReview({
92+
owner: context.repo.owner,
93+
repo: context.repo.repo,
94+
pull_number: context.payload.pull_request.number,
95+
event: 'REQUEST_CHANGES',
96+
body
97+
})
98+
}
99+
100+
- name: Dismiss old reviews
101+
if: ${{ github.event_name == 'pull_request_target' && steps.check.outcome == 'success' }}
102+
uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1
103+
with:
104+
script: |
105+
await Promise.all(
106+
(await github.paginate(github.rest.pulls.listReviews, {
107+
owner: context.repo.owner,
108+
repo: context.repo.repo,
109+
pull_number: context.payload.pull_request.number
110+
})).filter(review =>
111+
review.user.login == 'github-actions[bot]' &&
112+
review.state == 'CHANGES_REQUESTED'
113+
).map(async (review) => {
114+
await github.rest.pulls.dismissReview({
115+
owner: context.repo.owner,
116+
repo: context.repo.repo,
117+
pull_number: context.payload.pull_request.number,
118+
review_id: review.id,
119+
message: 'All cherry-picks are good now, thank you!'
120+
})
121+
await github.graphql(`mutation($node_id:ID!) {
122+
minimizeComment(input: {
123+
classifier: RESOLVED,
124+
subjectId: $node_id
125+
})
126+
{ clientMutationId }
127+
}`, { node_id: review.node_id })
128+
})
129+
)

‎.github/workflows/dismissed-review.yml‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
name: Dismissed Review
2+
3+
on:
4+
pull_request_review:
5+
types: [dismissed]
6+
7+
permissions:
8+
pull-requests: write
9+
10+
jobs:
11+
# The check-cherry-picks workflow creates review comments,
12+
# that should sometimes be manually dismissed.
13+
# When a CI-generated review is dismissed, this job automatically
14+
# minimizes it, to prevent it from cluttering the PR.
15+
minimize:
16+
name: Minimize as resolved
17+
if: github.event.review.user.login == 'github-actions[bot]'
18+
runs-on: ubuntu-24.04-arm
19+
steps:
20+
- uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1
21+
with:
22+
script: |
23+
await github.graphql(`mutation($node_id:ID!) {
24+
minimizeComment(input: {
25+
classifier: RESOLVED,
26+
subjectId: $node_id
27+
})
28+
{ clientMutationId }
29+
}`, { node_id: context.payload.review.node_id })
30+

‎ci/check-cherry-picks.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
This report is automatically generated by the `check-cherry-picks` CI workflow.
2+
3+
Some of the commits in this PR have not been cherry-picked exactly and require the author's and reviewer's attention.
4+
5+
Please make sure to follow the [backporting guidelines](https://github.com/NixOS/nixpkgs/blob/master/CONTRIBUTING.md#how-to-backport-pull-requests) and cherry-pick with the `-x` flag. This requires changes to go to the unstable branches (`master` / `staging`) first, before backporting them.
6+
7+
Occasionally, it is not possible to cherry-pick exactly the same patch. This most frequently happens when resolving merge conflicts while cherry-picking or when updating minor versions of packages which have already advanced to the next major on unstable. If you need to merge this PR despite the warnings, please [dismiss](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/dismissing-a-pull-request-review) this review.

‎ci/check-cherry-picks.sh‎

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,14 @@
33

44
set -euo pipefail
55

6-
if [ $# != "2" ] ; then
7-
echo "usage: check-cherry-picks.sh base_rev head_rev"
6+
if [[ $# != "2" && $# != "3" ]] ; then
7+
echo "usage: check-cherry-picks.sh base_rev head_rev [markdown_file]"
88
exit 2
99
fi
1010

11+
markdown_file="$(realpath ${3:-/dev/null})"
12+
[ -v 3 ] && rm -f "$markdown_file"
13+
1114
# Make sure we are inside the nixpkgs repo, even when called from outside
1215
cd "$(dirname "${BASH_SOURCE[0]}")"
1316

@@ -34,6 +37,18 @@ log() {
3437
fi
3538

3639
echo "${prefix[$type]}$@"
40+
41+
# Only logging errors and warnings, which allows comparing the markdown file
42+
# between pushes to the PR. Even if a new, proper cherry-pick, commit is added
43+
# it won't change the markdown file's content and thus not trigger another comment.
44+
if [ "$type" != "success" ]; then
45+
local -A alert
46+
alert[warning]="WARNING"
47+
alert[error]="CAUTION"
48+
echo >> $markdown_file
49+
echo "> [!${alert[$type]}]" >> $markdown_file
50+
echo "> $@" >> $markdown_file
51+
fi
3752
}
3853

3954
endgroup() {
@@ -58,9 +73,7 @@ while read -r new_commit_sha ; do
5873
)
5974
if [ -z "$original_commit_sha" ] ; then
6075
endgroup
61-
log error "Couldn't locate original commit hash in message"
62-
echo "Note this should not necessarily be treated as a hard fail, but a reviewer's attention should" \
63-
"be drawn to it and github actions have no way of doing that but to raise a 'failure'"
76+
log warning "Couldn't locate original commit hash in message of $new_commit_sha."
6477
problem=1
6578
continue
6679
fi
@@ -90,13 +103,19 @@ while read -r new_commit_sha ; do
90103
if $range_diff_common --no-color 2> /dev/null | grep -E '^ {4}[+-]{2}' > /dev/null ; then
91104
log success "$original_commit_sha present in branch $picked_branch"
92105
endgroup
93-
log warning "Difference between $new_commit_sha and original $original_commit_sha may warrant inspection:"
106+
log warning "Difference between $new_commit_sha and original $original_commit_sha may warrant inspection."
94107

95108
# First line contains commit SHAs, which we already printed.
96109
$range_diff_common --color | tail -n +2
97110

98-
echo "Note this should not necessarily be treated as a hard fail, but a reviewer's attention should" \
99-
"be drawn to it and github actions have no way of doing that but to raise a 'failure'"
111+
echo -e "> <details><summary>Show diff</summary>\n>" >> $markdown_file
112+
echo '> ```diff' >> $markdown_file
113+
# The output of `git range-diff` is indented with 4 spaces, which we need to match with the
114+
# code blocks indent to get proper syntax highlighting on GitHub.
115+
$range_diff_common | tail -n +2 | sed -Ee 's/^ {4}/> /g' >> $markdown_file
116+
echo '> ```' >> $markdown_file
117+
echo "> </details>" >> $markdown_file
118+
100119
problem=1
101120
else
102121
log success "$original_commit_sha present in branch $picked_branch"
@@ -112,7 +131,7 @@ while read -r new_commit_sha ; do
112131
done
113132

114133
endgroup
115-
log error "$original_commit_sha not found in any pickable branch"
134+
log error "$original_commit_sha given in $new_commit_sha not found in any pickable branch."
116135

117136
problem=1
118137
done <<< "$commits"

0 commit comments

Comments
 (0)