Skip to content

refactor(ci): extract shared helpers from description-check scripts #5486

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rudra496 wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

refactor(ci): extract shared helpers from description-check scripts #5486

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 21 additions & 70 deletions .github/scripts/check-issue-description.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,26 +1,22 @@
// @ts-check
'use strict';

const { section } = require('./lib/markdown');
const { addLabel, dropLabel } = require('./lib/labels');
const { upsertComment, deleteComment } = require('./lib/comment');

/** @param {{ github: import('@octokit/rest').Octokit, context: import('@actions/github').context, core: import('@actions/core') }} */
module.exports = async ({ github, context, core }) => {
const issue = context.payload.issue;
const body = (issue.body || '').trim();
const labels = issue.labels.map(l => l.name);
const owner = context.repo.owner;
const repo = context.repo.repo;
const num = issue.number;

const isBug = labels.includes('bug');
const isFeature = labels.includes('enhancement');

// Extract a Section's text, stripping HTML comments. Matches any heading
// depth (#, ##, ###, …) so a manually-written body isn't penalised for
// using a different number of hashes than the issue form generates.
function section(heading) {
const re = new RegExp(`#+\\s+${heading}\\s*([\\s\\S]*?)(?=\\n#+\\s+|$)`, 'i');
const m = body.match(re);
return m ? m[1].replace(/<!--[\s\S]*?-->/g, '').trim() : '';
}

const failures = [];

// ── Common: body must exist ───────────────────────────────────────────────
Expand All @@ -41,49 +37,49 @@ module.exports = async ({ github, context, core }) => {
break;

case 'bug': {
if (!section('Install Method')) {
if (!section(body, 'Install Method')) {
failures.push('**Install Method** — select how you installed Odysseus');
}

if (!section('Operating System')) {
if (!section(body, 'Operating System')) {
failures.push('**Operating System** — select your OS');
}

const stepsText = section('Steps to Reproduce');
const stepsText = section(body, 'Steps to Reproduce');
if (!stepsText || !/\d+\.|[-*]/.test(stepsText)) {
failures.push('**Steps to Reproduce** — must include at least one numbered or bulleted step');
}

if (section('Expected Behaviour').length < 10) {
if (section(body, 'Expected Behaviour').length < 10) {
failures.push('**Expected Behaviour** — section is empty or too short');
}

if (section('Actual Behaviour').length < 10) {
if (section(body, 'Actual Behaviour').length < 10) {
failures.push('**Actual Behaviour** — section is empty or too short');
}
break;
}

case 'feature':
if (!section('Area')) {
if (!section(body, 'Area')) {
failures.push('**Area** — select which part of the application this affects');
}

if (section('Problem or Motivation').length < 20) {
if (section(body, 'Problem or Motivation').length < 20) {
failures.push(
'**Problem or Motivation** — section is empty or too short ' +
'(explain the concrete problem this solves)',
);
}

if (section('Proposed Solution').length < 20) {
if (section(body, 'Proposed Solution').length < 20) {
failures.push(
'**Proposed Solution** — section is empty or too short ' +
'(describe the change you want to see)',
);
}

if (!section('Are you willing to implement this\\?')) {
if (!section(body, 'Are you willing to implement this?')) {
failures.push('**Are you willing to implement this?** — select an option');
}
break;
Expand Down Expand Up @@ -122,55 +118,15 @@ module.exports = async ({ github, context, core }) => {
);
}

// ── Labels ────────────────────────────────────────────────────────────────
// These labels are expected to already exist in the repo — managing the
// repo's label set is the maintainer's job, not this workflow's. We check a
// label exists before applying it (issues.addLabels would otherwise silently
// create a missing label) and fail soft — warn and skip — if it's absent.
async function labelExists(name) {
try {
await github.rest.issues.getLabel({ owner, repo, name });
return true;
} catch (e) {
if (e.status === 404) return false;
throw e;
}
}

async function addLabel(name) {
if (await labelExists(name)) {
await github.rest.issues.addLabels({ owner, repo, issue_number: issue.number, labels: [name] });
} else {
core.warning(`Label "${name}" does not exist in the repo — skipping. Create it once to enable labelling.`);
}
}

async function dropLabel(name) {
try {
await github.rest.issues.removeLabel({ owner, repo, issue_number: issue.number, name });
} catch (e) {
if (e.status !== 404 && e.status !== 410) throw e;
}
}

// ── Find existing bot comment to update in-place ──────────────────────────
// ── Labels + Comment ─────────────────────────────────────────────────────
const MARKER = '<!-- issue-description-check -->';
const { data: comments } = await github.rest.issues.listComments({
owner, repo, issue_number: issue.number,
});
const existing = comments.find(c => c.user.type === 'Bot' && c.body.includes(MARKER));

const LABEL_BAD = 'needs more info';
const LABEL_GOOD = 'ready for review';

if (failures.length === 0) {
if (existing) {
await github.rest.issues.deleteComment({ owner, repo, comment_id: existing.id });
}

await dropLabel(LABEL_BAD);
await addLabel(LABEL_GOOD);

await deleteComment(github, owner, repo, num, MARKER);
await dropLabel(github, owner, repo, num, LABEL_BAD);
await addLabel(github, core, owner, repo, num, LABEL_GOOD);
} else {
const list = failures.map(f => `- ${f}`).join('\n');
const commentBody = [
Expand All @@ -182,14 +138,9 @@ module.exports = async ({ github, context, core }) => {
'_This comment is deleted automatically once all sections are complete._',
].join('\n');

if (existing) {
await github.rest.issues.updateComment({ owner, repo, comment_id: existing.id, body: commentBody });
} else {
await github.rest.issues.createComment({ owner, repo, issue_number: issue.number, body: commentBody });
}

await dropLabel(LABEL_GOOD);
await addLabel(LABEL_BAD);
await upsertComment(github, owner, repo, num, MARKER, commentBody);
await dropLabel(github, owner, repo, num, LABEL_GOOD);
await addLabel(github, core, owner, repo, num, LABEL_BAD);

core.setFailed(`Issue description has ${failures.length} issue(s) — see bot comment for details.`);
}
Expand Down
75 changes: 11 additions & 64 deletions .github/scripts/check-pr-description.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
// @ts-check
'use strict';

const { section } = require('./lib/markdown');
const { swapLabel } = require('./lib/labels');
const { findComment, deleteComment, upsertComment } = require('./lib/comment');

/** @param {{ github: import('@octokit/rest').Octokit, context: import('@actions/github').context, core: import('@actions/core') }} */
module.exports = async ({ github, context, core }) => {
const body = context.payload.pull_request.body || '';
Expand All @@ -9,29 +13,17 @@ module.exports = async ({ github, context, core }) => {
const owner = context.repo.owner;
const repo = context.repo.repo;

// Strip HTML comments so placeholder text does not count as content.
function strip(text) {
return (text ?? '').replace(/<!--[\s\S]*?-->/g, '').trim();
}

// Extract the text content of a Section. Matches any heading depth (#, ##,
// ###, …) so the check doesn't break if the template's heading level changes.
function section(heading) {
const m = body.match(new RegExp(`#+\\s+${heading}[\\s\\S]*?(?=\\n#+\\s+|$)`, 'i'));
return strip(m?.[0].replace(new RegExp(`#+\\s+${heading}`, 'i'), '') ?? '');
}

const problems = [];

// 1. Summary must be filled in.
if (section('Summary').length < 20) {
if (section(body, 'Summary').length < 20) {
problems.push('**Summary** is empty or too short — describe what changed and why.');
}

// 2. Linked Issue must reference a real issue. Accept a bare #NNN, a closing
// keyword + #NNN, or a full issue URL (e.g. .../issues/123) — the strict
// keyword-prefixed form previously false-flagged correctly-linked PRs.
const linkedSection = section('Linked Issue');
const linkedSection = section(body, 'Linked Issue');
const hasIssueRef = /#\d+\b/.test(linkedSection) || /\/issues\/\d+/.test(linkedSection);
if (!linkedSection || !hasIssueRef) {
problems.push('**Linked Issue** — add a reference like `Fixes #NNN`, a bare `#NNN`, or a link to the issue.');
Expand All @@ -51,21 +43,14 @@ module.exports = async ({ github, context, core }) => {
// 5. How to Test must contain enough real detail for a reviewer to act on.
// Any format is fine — numbered steps, prose, the commands you ran, or a
// code block — so we only require non-trivial content, not a specific shape.
const howTo = section('How to Test');
const howTo = section(body, 'How to Test');
if (howTo.length < 30) {
problems.push('**How to Test** — explain how a reviewer can verify this change. Numbered steps, the commands you ran, or a short code block all work — give a sentence or two of real detail (not just "tested locally").');
}

// ── Comment ──────────────────────────────────────────────────────────────
const comments = await github.paginate(github.rest.issues.listComments, {
owner, repo, issue_number: prNum, per_page: 100,
});
const existing = comments.find(c => (c.body ?? '').includes(MARKER));

if (problems.length === 0) {
if (existing) {
await github.rest.issues.deleteComment({ owner, repo, comment_id: existing.id });
}
await deleteComment(github, owner, repo, prNum, MARKER);
} else {
const commentBody = [
MARKER,
Expand All @@ -79,52 +64,14 @@ module.exports = async ({ github, context, core }) => {
'_This comment is deleted automatically once all sections are complete._',
].join('\n');

if (existing) {
await github.rest.issues.updateComment({ owner, repo, comment_id: existing.id, body: commentBody });
} else {
await github.rest.issues.createComment({ owner, repo, issue_number: prNum, body: commentBody });
}
await upsertComment(github, owner, repo, prNum, MARKER, commentBody);
}

// ── Labels ────────────────────────────────────────────────────────────────
// These labels are expected to already exist in the repo — managing the
// repo's label set is the maintainer's job, not this workflow's. We check a
// label exists before applying it (issues.addLabels would otherwise silently
// create a missing label) and fail soft — warn and skip — if it's absent.
async function labelExists(name) {
try {
await github.rest.issues.getLabel({ owner, repo, name });
return true;
} catch (e) {
if (e.status === 404) return false;
throw e;
}
}

async function swapLabel(num, add, remove) {
if (await labelExists(add)) {
try {
await github.rest.issues.addLabels({ owner, repo, issue_number: num, labels: [add] });
} catch (e) {
// Fail soft on a token that can't write labels so a label permission
// problem never masks the actual description verdict.
if (e.status !== 403) throw e;
core.warning(`Could not add "${add}" — token lacks label write here; skipping.`);
}
} else {
core.warning(`Label "${add}" does not exist in the repo — skipping. Create it once to enable labelling.`);
}
try {
await github.rest.issues.removeLabel({ owner, repo, issue_number: num, name: remove });
} catch (e) {
if (e.status !== 404 && e.status !== 410 && e.status !== 403) throw e;
}
}

if (problems.length === 0) {
await swapLabel(prNum, 'ready for review', 'needs work');
await swapLabel(github, core, owner, repo, prNum, 'ready for review', 'needs work');
} else {
await swapLabel(prNum, 'needs work', 'ready for review');
await swapLabel(github, core, owner, repo, prNum, 'needs work', 'ready for review');
core.setFailed(`PR description has ${problems.length} issue(s) — see bot comment for details.`);
}
};
66 changes: 66 additions & 0 deletions .github/scripts/lib/comment.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
// @ts-check
'use strict';

/**
* Find-by-marker comment upsert/delete for description-check scripts.
*
* Extracted from check-pr-description.js and check-issue-description.js (#2453).
* Both scripts used the same paginated-lookup + marker-based upsert pattern.
*/

/**
* Find an existing bot comment containing the marker.
*
* Uses paginate so repos with 100+ comments on a single issue/PR are
* fully scanned (the default listComments cap is 30 per page, 100 max).
*
* @param {object} github — @octokit/rest instance
* @param {string} owner
* @param {string} repo
* @param {number} issue_number — PR or issue number
* @param {string} marker — the HTML comment marker to search for
* @returns {Promise<object|null>} the existing comment object, or null
*/
async function findComment(github, owner, repo, issue_number, marker) {
const comments = await github.paginate(github.rest.issues.listComments, {
owner, repo, issue_number, per_page: 100,
});
return comments.find(c => (c.body ?? '').includes(marker)) ?? null;
}

/**
* Upsert a comment: update if it exists (by marker), create if not.
*
* @param {object} github
* @param {string} owner
* @param {string} repo
* @param {number} issue_number
* @param {string} marker — HTML comment marker used to find the existing comment
* @param {string} body — the new comment body (should include the marker)
*/
async function upsertComment(github, owner, repo, issue_number, marker, body) {
const existing = await findComment(github, owner, repo, issue_number, marker);
if (existing) {
await github.rest.issues.updateComment({ owner, repo, comment_id: existing.id, body });
} else {
await github.rest.issues.createComment({ owner, repo, issue_number, body });
}
}

/**
* Delete a comment by marker, if it exists.
*
* @param {object} github
* @param {string} owner
* @param {string} repo
* @param {number} issue_number
* @param {string} marker
*/
async function deleteComment(github, owner, repo, issue_number, marker) {
const existing = await findComment(github, owner, repo, issue_number, marker);
if (existing) {
await github.rest.issues.deleteComment({ owner, repo, comment_id: existing.id });
}
}

module.exports = { findComment, upsertComment, deleteComment };
Loading