fix(contributors): address Copilot review comments and code quality issues

- Improve contributor-report.mjs based on Copilot comments and sonar
- Use fileURLToPath for Windows-safe main checks in contributor scripts
- Fix README badge URLs by removing duplicate style parameters
- Update graceful-shutdown.mjs to throw on process.exit failure for better error visibility

Generated-by: GitHub Copilot <copilot@github.com>
Signed-off-by: Ashley Childress <6563688+anchildress1@users.noreply.github.com>
This commit is contained in:
Ashley Childress
2026-01-14 23:47:04 -05:00
parent dcaff75b1a
commit f9e981e4cd
5 changed files with 92 additions and 141 deletions

View File

@@ -8,7 +8,7 @@ on:
jobs: jobs:
contributors: contributors:
runs-on: ubuntu-latest runs-on: ubuntu-latest
timeout-minutes: 12 timeout-minutes: 5
permissions: permissions:
contents: write contents: write
pull-requests: write pull-requests: write

View File

@@ -1,5 +1,5 @@
# 🤖 Awesome GitHub Copilot Customizations # 🤖 Awesome GitHub Copilot Customizations
[![Powered by Awesome Copilot](https://img.shields.io/badge/Powered_by-Awesome_Copilot-blue?logo=githubcopilot)](https://aka.ms/awesome-github-copilot?style=flat-square) [![GitHub contributors from allcontributors.org](https://img.shields.io/github/all-contributors/github/awesome-copilot?style=flat-square&color=ee8449)](#contributors-) [![Powered by Awesome Copilot](https://img.shields.io/badge/Powered_by-Awesome_Copilot-blue?logo=githubcopilot)](https://aka.ms/awesome-github-copilot) [![GitHub contributors from allcontributors.org](https://img.shields.io/github/all-contributors/github/awesome-copilot?color=ee8449)](#contributors-)
A community created collection of custom agents, prompts, and instructions to supercharge your GitHub Copilot experience across different domains, languages, and use cases. A community created collection of custom agents, prompts, and instructions to supercharge your GitHub Copilot experience across different domains, languages, and use cases.

View File

@@ -8,6 +8,7 @@
import { execSync } from 'node:child_process'; import { execSync } from 'node:child_process';
import fs from 'node:fs'; import fs from 'node:fs';
import path from 'node:path'; import path from 'node:path';
import { fileURLToPath } from 'node:url';
import { import {
getContributionTypes, getContributionTypes,
getMissingContributors, getMissingContributors,
@@ -282,7 +283,7 @@ const printSummaryReport = (results) => {
console.log('\n' + '='.repeat(50)); console.log('\n' + '='.repeat(50));
}; };
if (process.argv[1] === (new URL(import.meta.url)).pathname) { if (process.argv[1] && fileURLToPath(import.meta.url) === path.resolve(process.argv[1])) {
try { try {
const results = await main(); const results = await main();
printSummaryReport(results); printSummaryReport(results);

View File

@@ -5,6 +5,7 @@
import { execSync } from 'node:child_process'; import { execSync } from 'node:child_process';
import fs from 'node:fs'; import fs from 'node:fs';
import path from 'node:path'; import path from 'node:path';
import { fileURLToPath } from 'node:url';
import { setupGracefulShutdown } from './utils/graceful-shutdown.mjs'; import { setupGracefulShutdown } from './utils/graceful-shutdown.mjs';
const DEFAULT_CMD_TIMEOUT = 30_000; // 30s const DEFAULT_CMD_TIMEOUT = 30_000; // 30s
@@ -59,11 +60,14 @@ export const TYPE_PATTERNS = {
], ],
maintenance: [ maintenance: [
'package*.json', 'package*.json',
'*.config.js', '*config*',
'tsconfig*.json' 'tsconfig*.json'
], ],
code: [ code: [
'**/*.{js,ts,mjs,cjs}', '**/*.js',
'**/*.ts',
'**/*.mjs',
'**/*.cjs',
'**/*.py' '**/*.py'
] ]
}; };
@@ -78,17 +82,25 @@ const globCache = new Map();
*/ */
export const globToRegExp = (pattern) => { export const globToRegExp = (pattern) => {
const DOUBLE_WILDCARD_PLACEHOLDER = '§§DOUBLE§§'; const DOUBLE_WILDCARD_PLACEHOLDER = '§§DOUBLE§§';
const replacements = [
{ pattern: /\\/g, replacement: '/' },
{ pattern: /\./g, replacement: String.raw`\.` },
{ pattern: /\*\*/g, replacement: DOUBLE_WILDCARD_PLACEHOLDER },
{ pattern: /\*/g, replacement: '[^/]*' },
{ pattern: new RegExp(DOUBLE_WILDCARD_PLACEHOLDER, 'g'), replacement: '.*' },
{ pattern: /\?/g, replacement: '.' },
{ pattern: /\//g, replacement: String.raw`\/` }
];
const normalized = replacements.reduce((acc, { pattern, replacement }) => acc.replace(pattern, replacement), String(pattern)); // Escape all regex-special characters except glob wildcards (*, ?, /),
// then translate glob syntax to regex.
// Note: This function intentionally supports only a small subset of glob syntax.
const regexSpecials = /[.+^${}()|[\]\\]/g;
let normalized = String(pattern);
// Normalize Windows-style separators to POSIX-style for matching.
normalized = normalized.replaceAll('\\', '/');
// Escape regex metacharacters so they are treated literally.
normalized = normalized.replaceAll(regexSpecials, (match) => `\\${match}`);
// Handle glob wildcards.
normalized = normalized.replaceAll('**', DOUBLE_WILDCARD_PLACEHOLDER);
normalized = normalized.replaceAll('*', '[^/]*');
normalized = normalized.replaceAll(DOUBLE_WILDCARD_PLACEHOLDER, '.*');
normalized = normalized.replaceAll('?', '.');
return new RegExp(`^${normalized}$`); return new RegExp(`^${normalized}$`);
}; };
@@ -113,7 +125,7 @@ export const matchGlob = (filePath, pattern) => {
return false; return false;
} }
const normalized = filePath.replace(/\\/g, '/'); const normalized = filePath.replaceAll('\\', '/');
return regexp.test(normalized); return regexp.test(normalized);
}; };
@@ -133,7 +145,7 @@ export const isAutoGeneratedFile = (filePath) => {
* @returns {string|null} * @returns {string|null}
*/ */
export const getFileContributionType = (filePath) => { export const getFileContributionType = (filePath) => {
const normalized = filePath.replace(/\\/g, '/'); const normalized = filePath.replaceAll('\\', '/');
for (const [type, patterns] of Object.entries(TYPE_PATTERNS)) { for (const [type, patterns] of Object.entries(TYPE_PATTERNS)) {
if (patterns.some((pattern) => matchGlob(normalized, pattern))) { if (patterns.some((pattern) => matchGlob(normalized, pattern))) {
@@ -267,17 +279,33 @@ export const getMissingContributors = () => {
* @returns {string} * @returns {string}
*/ */
const getGitHubRepo = () => { const getGitHubRepo = () => {
const parseRepoFromRemoteUrl = (remoteUrl) => {
const url = String(remoteUrl || '').trim();
if (!url) return null;
// Supports:
// - git@github.com:owner/repo.git
// - ssh://git@github.com/owner/repo.git
// - https://github.com/owner/repo.git
// - https://github.com/owner/repo
const regex = /github\.com[/:]([^/]+)\/([^/?#]+?)(?:\.git)?(?:[/?#]|$)/;
const match = regex.exec(url);
if (!match) return null;
return `${match[1]}/${match[2]}`;
};
try { try {
const upstreamUrl = execSync('git config --get remote.upstream.url', { const upstreamUrl = execSync('git config --get remote.upstream.url', {
encoding: 'utf8', encoding: 'utf8',
stdio: ['pipe', 'pipe', 'pipe'] stdio: ['pipe', 'pipe', 'pipe']
}).trim(); }).trim();
if (upstreamUrl) { if (upstreamUrl) {
const match = upstreamUrl.match(/github\.com:([^/]+)\/([^/]+?)(?:\.git)?$/); const repo = parseRepoFromRemoteUrl(upstreamUrl);
if (match) return `${match[1]}/${match[2]}`; if (repo) return repo;
} }
} catch (e) { } catch (e) {
console.debug('upstream not found, trying origin'); console.debug('upstream not found, trying origin', e?.message || e);
} }
try { try {
@@ -285,22 +313,15 @@ const getGitHubRepo = () => {
encoding: 'utf8', encoding: 'utf8',
stdio: ['pipe', 'pipe', 'pipe'] stdio: ['pipe', 'pipe', 'pipe']
}).trim(); }).trim();
const match = originUrl.match(/github\.com:([^/]+)\/([^/]+?)(?:\.git)?$/); const repo = parseRepoFromRemoteUrl(originUrl);
if (match) return `${match[1]}/${match[2]}`; if (repo) return repo;
} catch (e) { } catch (e) {
console.debug('origin not found, using default'); console.debug('origin not found, using default', e?.message || e);
} }
return 'github/awesome-copilot'; return 'github/awesome-copilot';
}; };
const CONTRIBUTION_TYPE_MAP = {
'instructions': { symbol: '🧭', description: 'The big AI prompt recipes (Copilot instruction sets)' },
'prompts': { symbol: '⌨️', description: 'One-shot or reusable user-level prompts' },
'agents': { symbol: '🎭', description: 'Defined Copilot personalities / roles' },
'collections': { symbol: '🎁', description: 'Bundled thematic sets (e.g., "Copilot for Docs")' }
};
/** /**
* Fetch merged PRs for a GitHub username using the GH CLI and filter files. * Fetch merged PRs for a GitHub username using the GH CLI and filter files.
* @param {string} username * @param {string} username
@@ -445,12 +466,14 @@ export const generateMarkdownReport = (reports, missingCount = 0) => {
const lines = []; const lines = [];
lines.push('# Missing Contributors Report'); lines.push(
lines.push(''); '# Missing Contributors Report',
lines.push(`Generated (ISO): ${nowIso}`); '',
lines.push(''); `Generated (ISO): ${nowIso}`,
lines.push(`Missing contributors: ${missingCount}`); '',
lines.push(''); `Missing contributors: ${missingCount}`,
''
);
for (const report of reports) { for (const report of reports) {
lines.push(`## @${report.username}`); lines.push(`## @${report.username}`);
@@ -464,13 +487,15 @@ export const generateMarkdownReport = (reports, missingCount = 0) => {
}); });
if (prs.length === 0) { if (prs.length === 0) {
lines.push(''); lines.push(
lines.push('_No eligible PRs found._'); '',
lines.push(''); '_No eligible PRs found._',
lines.push(`Alternate CLI: \`npx all-contributors add ${report.username} ${computeTypesArg(report)}\``); '',
lines.push(''); `Alternate CLI: \`npx all-contributors add ${report.username} ${computeTypesArg(report)}\``,
lines.push('---'); '',
lines.push(''); '---',
''
);
continue; continue;
} }
@@ -483,101 +508,30 @@ export const generateMarkdownReport = (reports, missingCount = 0) => {
const url = String(pr.prUrl ?? ''); const url = String(pr.prUrl ?? '');
const comment = `@all-contributors please add @${report.username} for ${prTypesArg}`; const comment = `@all-contributors please add @${report.username} for ${prTypesArg}`;
// PR line lines.push(
lines.push(`[#${pr.prNumber}](${url}) ${title}`); `[#${pr.prNumber}](${url}) ${title}`,
// fenced single-line comment snippet for this PR (plaintext as requested) '```plaintext',
lines.push('```plaintext'); comment,
lines.push(comment); '```'
lines.push('```'); );
} }
lines.push(''); lines.push(
lines.push('### Alternate CLI Command'); '',
lines.push(''); '### Alternate CLI Command',
lines.push('```bash'); '',
lines.push(`npx all-contributors add ${report.username} ${computeTypesArg(report)}`); '```bash',
lines.push('```'); `npx all-contributors add ${report.username} ${computeTypesArg(report)}`,
lines.push(''); '```',
lines.push('---'); '',
lines.push(''); '---',
''
);
} }
return `${lines.join('\n')}\n`; return `${lines.join('\n')}\n`;
}; };
/**
* Check whether a PR already contains an all-contributors bot comment.
* @param {number} prNumber
* @returns {boolean}
*/
export const hasExistingAllContributorsComment = (prNumber) => {
try {
const repo = getGitHubRepo();
const json = execSync(`gh pr view ${prNumber} --repo ${repo} --json comments`, {
encoding: 'utf8',
stdio: ['pipe', 'pipe', 'pipe'],
timeout: DEFAULT_CMD_TIMEOUT
});
const data = JSON.parse(json);
const comments = data?.comments?.nodes || data?.comments || [];
return comments.some((comment) => comment?.body?.includes(`@all-contributors`));
} catch (error) {
console.warn(`⚠️ Unable to inspect comments for PR #${prNumber}: ${error.message}`);
return false;
}
};
/**
* Post a comment to a PR using the GH CLI.
* @param {number} prNumber
* @param {string} body
* @returns {boolean}
*/
export const postCommentOnPr = (prNumber, body) => {
try {
const repo = getGitHubRepo();
execSync(`gh pr comment ${prNumber} --repo ${repo} --body "${body.replace(/"/g, '\\"')}"`, {
encoding: 'utf8',
stdio: ['pipe', 'inherit', 'inherit'],
timeout: DEFAULT_CMD_TIMEOUT
});
console.log(`💬 Posted recommendation comment on PR #${prNumber}`);
return true;
} catch (error) {
console.warn(`⚠️ Failed to post comment on PR #${prNumber}: ${error.message}`);
return false;
}
};
/**
* Post suggested all-contributors comments to PRs for a collection of reports.
* @param {Array<object>} reports
*/
export const autoAddCommentsToReports = (reports) => {
for (const report of reports) {
for (const pr of report.prs) {
if (hasExistingAllContributorsComment(pr.prNumber)) {
console.log(`💬 Skipping PR #${pr.prNumber} for @${report.username} — comment already present`);
continue;
}
const types = pr.contributionTypes.map(t => '`' + t + '`').join(', ');
const commentLines = [
`Thanks for the contribution @${report.username}!`,
'',
`We detected contribution categories for this PR: ${types || '`code`'}.`,
'',
`@all-contributors please add @${report.username} for ${pr.contributionTypes.join(', ')}`
];
const body = commentLines.join('\n');
postCommentOnPr(pr.prNumber, body);
}
}
};
const main = () => { const main = () => {
try { try {
// gh CLI can use either its own authenticated session or token env vars. // gh CLI can use either its own authenticated session or token env vars.
@@ -587,7 +541,6 @@ const main = () => {
} }
const args = new Set(process.argv.slice(2)); const args = new Set(process.argv.slice(2));
const autoAdd = args.has('--auto-add-pr-comments');
const includeAllFiles = args.has('--include-all-pr-files'); const includeAllFiles = args.has('--include-all-pr-files');
const contributors = getMissingContributors(); const contributors = getMissingContributors();
@@ -605,16 +558,12 @@ const main = () => {
console.log(`Report saved to: ${outputPath}`); console.log(`Report saved to: ${outputPath}`);
if (autoAdd) {
autoAddCommentsToReports(reports);
}
} catch (error) { } catch (error) {
console.error('Error generating report:', error); console.error('Error generating report:', error);
process.exit(1); process.exit(1);
} }
}; };
if (process.argv[1] === (new URL(import.meta.url)).pathname) { if (process.argv[1] && fileURLToPath(import.meta.url) === path.resolve(process.argv[1])) {
main(); main();
} }

View File

@@ -26,8 +26,9 @@ export const setupGracefulShutdown = (name, { exitCode = 1 } = {}) => {
try { try {
process.exit(exitCode); process.exit(exitCode);
} catch (e) { } catch (e) {
// process.exit may not be desirable in some test harnesses; swallow errors // If process.exit is stubbed or overridden (e.g. in tests), surface the failure.
console.warn(`${name}: process.exit failed:`, e?.message); console.error(`${name}: process.exit failed:`, e?.message || e);
throw e;
} }
}; };