Skip to content

Commit 109731f

Browse files
kamal-kaur04kamal-kaur04
authored
Merge pull request #57 from browserstack/ci/port-pr-review-workflows-from-ruby-sdk
ci: port PR template, ready-for-review label + PR-review gate workflows from ruby-sdk
2 parents 114c7ba + e732090 commit 109731f

3 files changed

Lines changed: 427 additions & 0 deletions

File tree

‎.github/PULL_REQUEST_TEMPLATE.md‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
## What is this about?
2+
<!-- Give a small description related to the changes you have made -->
3+
4+
## Related Jira task/s
5+
<!-- Add associated JIRA links -->
6+
7+
## Release (mandatory for every PR — required for the `ready-for-review` label)
8+
<!-- CI gates the `ready-for-review` label on the **version bump** and the **internal release notes** below. Customer-facing notes + type are optional but strongly encouraged for the daily release. -->
9+
10+
**Version bump:** *(required — tick exactly one)*
11+
<!-- minor = ANY new feature/capability (backwards-compatible). patch = bug fix or trivial change only. New features mislabeled as patch is the common mistake — when unsure, choose minor. -->
12+
- [ ] minor (backwards-compatible feature)
13+
- [ ] patch (bug fix or other small change)
14+
15+
**Release notes type:** *(optional)*
16+
- [ ] New Feature
17+
- [ ] Bug Fix
18+
- [ ] Other Improvement
19+
20+
**Release notes (customer-facing):** *(optional but encouraged)*
21+
<!-- 1-3 bullets in customer language, e.g. "Fixed test results not being reported when using the CLI." -->
22+
-
23+
24+
**Release notes (internal):** *(required — engineer-facing; what actually changed / why)*
25+
<!-- REQUIRED: at least one non-empty bullet — the ready-for-review gate checks this. Dev-facing notes for the internal CHANGELOG.md (non-customer-facing). -->
26+
-
27+
28+
## Checklist
29+
- [ ] Ready to review
30+
- [ ] Has it been tested locally?
31+
32+
## PR Validations
33+
Run Tests: Comment RUN_TESTS to trigger sanity tests.

‎.github/workflows/ready-for-review-label.yml‎

Lines changed: 220 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,220 @@
1+
name: Ready for Review Label
2+
3+
on:
4+
pull_request:
5+
types: [edited, synchronize, labeled, unlabeled]
6+
7+
# Coalesce runs per PR. Label add/remove re-fires this workflow; cancel-in-progress
8+
# drops the superseded run so only the latest state is evaluated.
9+
concurrency:
10+
group: ready-for-review-${{ github.event.pull_request.number }}
11+
cancel-in-progress: true
12+
13+
jobs:
14+
check-ready:
15+
# NO job-level `if`. Because Req 3 makes the JOB FAILURE the gate, this job MUST
16+
# always conclude red/green and never "skipped" — a skipped required check reads
17+
# as passing on GitHub, so an `if` that skips on an unrelated label event would
18+
# silently flip the hard gate from red to green. The gate runs on every triggering
19+
# event and does its own label filtering internally (it only ever removes the
20+
# ready-for-review label and computes purely from the PR's current labels).
21+
runs-on: ubuntu-latest
22+
permissions:
23+
pull-requests: write
24+
issues: write
25+
steps:
26+
- name: Validate ready-for-review label against the release gate (SDK-6104 #8)
27+
id: gate
28+
uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1
29+
with:
30+
script: |
31+
const body = context.payload.pull_request.body || '';
32+
// "Ready to review" is matched against the WHOLE body: the string is
33+
// unambiguous and lives in the Checklist section, not Release.
34+
const isCheckboxTicked = /- \[[xX]\] Ready to review/.test(body);
35+
const labels = context.payload.pull_request.labels.map(l => l.name);
36+
const hadLabel = labels.includes('ready-for-review');
37+
38+
// Anchor the bump count to the "## Release" section only. A bump tick
39+
// appearing elsewhere (description prose, another checklist, a quoted
40+
// template) must NOT satisfy the gate. Slice the body into heading blocks
41+
// at each "## " boundary and pick the Release block.
42+
const releaseBlock = body.split(/(?=^## )/m).find(s => /^##[ \t]*Release[ \t]*(\(|$)/m.test(s)) || '';
43+
const hasReleaseSection = releaseBlock !== '';
44+
// The free-form "Release notes (...)" subsections live INSIDE the Release
45+
// block (bold headers, not "## " headings) and may legitimately contain
46+
// checkbox-looking bullets (e.g. an internal note "- [x] patch the retry
47+
// path"). Count bump ticks only in the region BEFORE the first notes
48+
// header so notes text can never flip the count (mirrors the release
49+
// planner's scan scoping).
50+
const bumpRegion = releaseBlock.split(/^\*\*Release notes \(/m)[0];
51+
// The (?=\s|\(|$) lookahead matches a bump tick followed by whitespace, an
52+
// opening paren ("minor (...)"), or end-of-string. It rejects e.g. "minor-stub".
53+
const bumpTicks = [...bumpRegion.matchAll(/- \[[xX]\]\s*(minor|patch)(?=\s|\(|$)/g)].length;
54+
55+
// Internal release notes are REQUIRED (they feed the internal CHANGELOG.md):
56+
// the Release block must carry at least one non-empty bullet under
57+
// "**Release notes (internal):**". The header-line remainder (the italic
58+
// "*(required ...)*" annotation) and HTML comments are stripped first so
59+
// template scaffolding can never satisfy the check, and the empty
60+
// placeholder "- " does not count. Auto-generated release PRs (head branch
61+
// "release_x.y.z") are EXEMPT: the release workflow writes the CHANGELOG.md
62+
// entries itself from the constituent PRs' notes, so the release PR's own
63+
// subsection legitimately stays empty.
64+
const isReleasePr = /^release_\d+(\.\d+)*$/.test(context.payload.pull_request.head.ref);
65+
const internalSplit = releaseBlock.split(/^\*\*Release notes \(internal\):\*\*/m);
66+
const internalRegion = internalSplit.length > 1
67+
? internalSplit[1].replace(/^[^\n]*\n?/, '').split(/^\*\*/m)[0].replace(/<!--[\s\S]*?-->/g, '')
68+
: '';
69+
const internalFilled = /^[ \t]*[-*][ \t]*\S/m.test(internalRegion);
70+
const internalOk = isReleasePr || internalFilled;
71+
72+
// reasons -> full sentences for the PR comment.
73+
// reasonBits -> short single-line fragments for step outputs / Slack.
74+
const reasons = [];
75+
const reasonBits = [];
76+
if (!hasReleaseSection) {
77+
const templateUrl = `${context.payload.repository.html_url}/blob/${context.payload.repository.default_branch}/.github/PULL_REQUEST_TEMPLATE.md`;
78+
reasons.push(`The PR body has no \`## Release\` section. Apply the [repo PR template](${templateUrl}) to the PR body, then tick exactly one version bump.`);
79+
reasonBits.push('missing ## Release section');
80+
} else if (bumpTicks === 0) {
81+
reasons.push('Tick exactly one version bump (minor or patch) in the Release section.');
82+
reasonBits.push('no version bump ticked in ## Release');
83+
} else if (bumpTicks > 1) {
84+
reasons.push('Tick exactly ONE version bump (minor or patch), not multiple.');
85+
reasonBits.push(`expected exactly one version bump tick in ## Release, found ${bumpTicks}`);
86+
} else if (!internalOk) {
87+
reasons.push('Fill in at least one non-empty bullet under `**Release notes (internal):**` in the Release section (engineer-facing — it feeds the internal CHANGELOG.md). The empty placeholder `- ` does not count.');
88+
reasonBits.push('internal release notes empty');
89+
}
90+
if (!isCheckboxTicked) {
91+
reasons.push('Tick `- [x] Ready to review` once the PR body is complete.');
92+
reasonBits.push('Ready-to-review checkbox not ticked');
93+
}
94+
95+
const bumpOk = bumpTicks === 1;
96+
const shouldHaveLabel = isCheckboxTicked && bumpOk && internalOk;
97+
98+
// This workflow NEVER ADDS the ready-for-review label — the author applies
99+
// it manually. It only REMOVES the label when the label is present but the
100+
// body does not pass the release gate. The "Require label" step below then
101+
// FAILS the check whenever the label is absent, making ready-for-review a
102+
// required, manually-applied merge gate.
103+
let removedNow = false;
104+
if (hadLabel && !shouldHaveLabel) {
105+
await github.rest.issues.removeLabel({
106+
...context.repo,
107+
issue_number: context.issue.number,
108+
name: 'ready-for-review'
109+
}).catch(() => {});
110+
removedNow = true;
111+
}
112+
// We never add, so the label is present at the end iff it was present and
113+
// we did not strip it.
114+
const labelPresent = hadLabel && !removedNow;
115+
116+
// Warning comment (single, marker-deduped): posted when the author has
117+
// CLAIMED readiness (ticked the checkbox) but the body fails, or when this
118+
// run removed the label. A WIP PR that never claimed readiness gets NO
119+
// comment — the failing check is the signal; we don't spam every draft.
120+
// Deleted once the body passes AND the label is present.
121+
// Emit step outputs FIRST — the notify and enforce steps depend on them,
122+
// and they must be set regardless of the (advisory, best-effort) warning
123+
// comment lifecycle below.
124+
core.setOutput('removed', removedNow ? 'true' : 'false');
125+
core.setOutput('reason', reasonBits.join('; ') || 'PR body passes the release gate');
126+
core.setOutput('author', context.payload.pull_request.user.login);
127+
core.setOutput('label_present', labelPresent ? 'true' : 'false');
128+
129+
// Warning comment lifecycle — ADVISORY ONLY, wrapped in try/catch so a transient
130+
// GitHub comments-API error can neither (a) fail the check on a passing PR nor
131+
// (b) skip the Slack notify step (the gate step must stay green so the later
132+
// step `if`s evaluate). Posted when the author CLAIMED readiness (ticked the
133+
// checkbox) but the body fails, or when this run removed the label. A WIP PR that
134+
// never claimed readiness gets NO comment. Deleted once the body passes AND the
135+
// label is present.
136+
const marker = '<!-- sdk-6104:release-section-incomplete -->';
137+
const shouldWarn = removedNow || (isCheckboxTicked && !(bumpOk && internalOk));
138+
let warningBody = '';
139+
if (shouldWarn) {
140+
const headline = removedNow
141+
? '⚠️ The `ready-for-review` label was removed because the PR body does not pass the release gate:'
142+
: '⚠️ The PR body claims it is ready to review but does not pass the release gate:';
143+
warningBody = marker + '\n' + headline + '\n- ' + reasons.join('\n- ') +
144+
'\n\nHow to get (and keep) the `ready-for-review` label:\n' +
145+
'1. Make sure the PR body follows the repo PR template (all sections present).\n' +
146+
'2. In the `## Release` section, tick exactly one version bump (`minor` or `patch`).\n' +
147+
'3. Fill in at least one bullet under `**Release notes (internal):**` (engineer-facing; feeds the internal CHANGELOG.md).\n' +
148+
'4. Tick `- [x] Ready to review` in the checklist.\n' +
149+
'5. Apply the `ready-for-review` label yourself — this workflow does NOT add it for you; it only removes it if the body stops passing, and the check stays red until the label is present.';
150+
}
151+
try {
152+
const comments = await github.paginate(github.rest.issues.listComments, {
153+
...context.repo,
154+
issue_number: context.issue.number,
155+
per_page: 100
156+
});
157+
const existingWarning = comments.find(c => (c.body || '').includes(marker));
158+
if (shouldWarn && !existingWarning) {
159+
await github.rest.issues.createComment({
160+
...context.repo,
161+
issue_number: context.issue.number,
162+
body: warningBody
163+
});
164+
} else if (shouldWarn && existingWarning && existingWarning.body !== warningBody) {
165+
await github.rest.issues.updateComment({
166+
...context.repo,
167+
comment_id: existingWarning.id,
168+
body: warningBody
169+
});
170+
} else if (labelPresent && existingWarning) {
171+
await github.rest.issues.deleteComment({
172+
...context.repo,
173+
comment_id: existingWarning.id
174+
});
175+
}
176+
} catch (e) {
177+
console.log(`warning-comment lifecycle skipped (non-fatal): ${e.message}`);
178+
}
179+
180+
- name: Notify Slack on label removal
181+
if: steps.gate.outputs.removed == 'true'
182+
env:
183+
SLACK_WEBHOOK_URL: ${{ secrets.SLACK_PR_SLA_WEBHOOK }}
184+
PR_URL: ${{ github.event.pull_request.html_url }}
185+
PR_REF: "${{ github.repository }}#${{ github.event.pull_request.number }}"
186+
REASON: ${{ steps.gate.outputs.reason }}
187+
AUTHOR: ${{ steps.gate.outputs.author }}
188+
run: |
189+
if [ -z "$SLACK_WEBHOOK_URL" ]; then
190+
echo "SLACK_PR_SLA_WEBHOOK not configured — skipping Slack notification"
191+
exit 0
192+
fi
193+
# The author identity goes in the message TEXT and github_username is OMITTED
194+
# from the payload. The SLACK_PR_SLA_WEBHOOK Workflow-Builder workflow resolves
195+
# github_username -> @mention via a List lookup that ERRORS (dropping the whole
196+
# run, so nothing posts) when the login is not in the list — e.g. an external
197+
# PR author. Omitting github_username skips that lookup so the message ALWAYS
198+
# surfaces (without an @mention). Plain text + bare URL (no mrkdwn link syntax).
199+
text=":warning: ready-for-review was removed from ${PR_REF} (author: ${AUTHOR}) — ${REASON}. ${PR_URL} — fix the PR body per the repo PR template, then re-apply the label."
200+
payload=$(jq -n --arg text "$text" '{text: $text}')
201+
curl -fsS --max-time 10 -X POST -H 'Content-Type: application/json' \
202+
-d "$payload" "$SLACK_WEBHOOK_URL" \
203+
|| echo "Slack notify failed (non-fatal)"
204+
205+
- name: Require the ready-for-review label
206+
# Hard gate: this step fails the check whenever the label is absent (a WIP PR,
207+
# a never-applied label, or one this run just removed). It runs after the notify
208+
# step (the gate step does not fail, so the notify is never skipped). Green only
209+
# when the label is present — i.e. the author applied it AND the body passes
210+
# (otherwise the gate step above would have removed it).
211+
if: steps.gate.outputs.label_present != 'true'
212+
run: |
213+
echo "::error::The 'ready-for-review' label is required and is not present on this PR."
214+
echo "Apply the 'ready-for-review' label once the PR body passes the release gate:"
215+
echo " - the PR template is complete,"
216+
echo " - the '## Release' section has exactly one version bump (minor or patch),"
217+
echo " - '**Release notes (internal):**' has at least one non-empty bullet,"
218+
echo " - and '- [x] Ready to review' is ticked."
219+
echo "This workflow does not add the label for you; the check stays red until the label is present."
220+
exit 1

0 commit comments

Comments
 (0)