Skip to content

Commit 3c0321f

Browse files
Address PR title prefix review feedback
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 5a5e4af commit 3c0321f

6 files changed

Lines changed: 498 additions & 106 deletions

File tree

.github/pull_request_template.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,10 @@ Please help reviewers and future users, providing the following information:
2525
### Related Issue
2626

2727
<!-- Which issue does this PR fix? Link it using a GitHub closing keyword so it is
28-
closed automatically when this PR is merged, e.g. "Fixes #123" or "Closes #123".
29-
PRs that are not linked to an issue may be closed, no matter how valid the change is.
30-
Also check if there are not open PR's for this issue already,
31-
then please explain how this PR is different.-->
28+
closed automatically when this PR is merged, e.g. "Fixes #123" or "Closes #123".
29+
PRs that are not linked to an issue may be closed, no matter how valid the change is.
30+
Also check whether an open PR already exists for this issue; if so,
31+
explain how this PR is different. -->
3232

3333
Fixes #
3434

@@ -40,4 +40,4 @@ Fixes #
4040
- [ ] All unit tests pass, and I have added new tests where possible
4141
- [ ] The PR follows the [Contribution Guidelines](https://github.com/microsoft/agent-framework/blob/main/CONTRIBUTING.md)
4242
- [ ] This PR is linked to an issue and there is no other open PR for this issue (see Related Issue above).
43-
- [ ] **This is not a breaking change.** If it _is_ a breaking change, add the `breaking change` label (or add a "[BREAKING]" prefix to the title) — a workflow keeps the label and title prefix in sync automatically.
43+
- [x] **This is not a breaking change.** If it _is_ a breaking change, add the `breaking change` label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.

.github/scripts/title_prefix.js

Lines changed: 252 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,252 @@
1+
// Copyright (c) Microsoft. All rights reserved.
2+
3+
const BREAKING_CHANGE_LABEL = 'breaking change';
4+
const BREAKING_PREFIX = '[BREAKING]';
5+
6+
const DEFAULT_PREFIX_LABELS = Object.freeze({
7+
python: 'Python',
8+
'.NET': '.NET',
9+
});
10+
11+
const DEFAULT_BRACKET_PREFIX_LABELS = Object.freeze({
12+
[BREAKING_CHANGE_LABEL]: BREAKING_PREFIX,
13+
});
14+
15+
function escapeRegExp(value) {
16+
return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
17+
}
18+
19+
function getMatchingValueByKey(valuesByKey, keyToFind) {
20+
const matchingKey = Object.keys(valuesByKey).find((key) => key.toLowerCase() === keyToFind.toLowerCase());
21+
return matchingKey === undefined ? null : valuesByKey[matchingKey];
22+
}
23+
24+
function getPrefixPattern(prefixes) {
25+
return prefixes.map(escapeRegExp).join('|');
26+
}
27+
28+
function canonicalizePrefix(prefix, prefixes) {
29+
return prefixes.find((knownPrefix) => knownPrefix.toLowerCase() === prefix.toLowerCase()) ?? prefix;
30+
}
31+
32+
function normalizeLeadingBracketPrefix(title, bracketPrefixes) {
33+
const bracketPattern = getPrefixPattern(bracketPrefixes);
34+
if (!bracketPattern) {
35+
return title;
36+
}
37+
38+
const leadingBracketPrefix = new RegExp(`^(${bracketPattern})(?=\\s|$)`, 'i');
39+
return title.replace(
40+
leadingBracketPrefix,
41+
(bracketPrefix) => canonicalizePrefix(bracketPrefix, bracketPrefixes),
42+
);
43+
}
44+
45+
function parseLeadingTitlePrefix(title, titlePrefixes) {
46+
const titlePrefixPattern = getPrefixPattern(titlePrefixes);
47+
if (!titlePrefixPattern) {
48+
return null;
49+
}
50+
51+
const match = title.match(new RegExp(`^(${titlePrefixPattern}):\\s*`, 'i'));
52+
if (!match) {
53+
return null;
54+
}
55+
56+
return {
57+
prefix: canonicalizePrefix(match[1], titlePrefixes),
58+
rest: title.slice(match[0].length).trimStart(),
59+
};
60+
}
61+
62+
function removeBracketPrefixToken(title, bracketPrefix) {
63+
const bracketPrefixPattern = escapeRegExp(bracketPrefix);
64+
return title
65+
.replace(new RegExp(`(^|\\s+)${bracketPrefixPattern}(?=\\s|$)`, 'ig'), '$1')
66+
.replace(/\s{2,}/g, ' ')
67+
.trim();
68+
}
69+
70+
function addTitlePrefix(title, prefix, bracketPrefixes = Object.values(DEFAULT_BRACKET_PREFIX_LABELS)) {
71+
const bracketPattern = getPrefixPattern(bracketPrefixes);
72+
const prefixPattern = escapeRegExp(prefix);
73+
74+
if (bracketPattern) {
75+
const bracketThenTitlePrefix = new RegExp(`^(${bracketPattern})(\\s+)(${prefixPattern})(?=:)`, 'i');
76+
if (bracketThenTitlePrefix.test(title)) {
77+
return title.replace(
78+
bracketThenTitlePrefix,
79+
(match, bracketPrefix, spacing) => `${canonicalizePrefix(bracketPrefix, bracketPrefixes)}${spacing}${prefix}`,
80+
);
81+
}
82+
83+
title = normalizeLeadingBracketPrefix(title, bracketPrefixes);
84+
}
85+
86+
if (!title.startsWith(`${prefix}: `)) {
87+
if (title.match(new RegExp(`^${prefixPattern}`, 'i'))) {
88+
return title.replace(new RegExp(`^${prefixPattern}`, 'i'), prefix);
89+
}
90+
91+
return `${prefix}: ${title}`;
92+
}
93+
94+
return title;
95+
}
96+
97+
function hasBracketPrefix(title, bracketPrefix, titlePrefixes = Object.values(DEFAULT_PREFIX_LABELS)) {
98+
const bracketPrefixPattern = escapeRegExp(bracketPrefix);
99+
const leadingBracketPrefix = new RegExp(`^${bracketPrefixPattern}(?=\\s|$)`, 'i');
100+
if (leadingBracketPrefix.test(title)) {
101+
return true;
102+
}
103+
104+
const leadingTitlePrefix = parseLeadingTitlePrefix(title, titlePrefixes);
105+
if (!leadingTitlePrefix) {
106+
return false;
107+
}
108+
109+
return leadingBracketPrefix.test(leadingTitlePrefix.rest);
110+
}
111+
112+
function addBracketPrefix(title, bracketPrefix, titlePrefixes = Object.values(DEFAULT_PREFIX_LABELS)) {
113+
const bracketPrefixPattern = escapeRegExp(bracketPrefix);
114+
const leadingBracketPrefix = new RegExp(`^${bracketPrefixPattern}(?=\\s|$)`, 'i');
115+
if (leadingBracketPrefix.test(title)) {
116+
return title.replace(leadingBracketPrefix, bracketPrefix);
117+
}
118+
119+
const leadingTitlePrefix = parseLeadingTitlePrefix(title, titlePrefixes);
120+
if (leadingTitlePrefix) {
121+
if (leadingBracketPrefix.test(leadingTitlePrefix.rest)) {
122+
const normalizedRest = leadingTitlePrefix.rest.replace(leadingBracketPrefix, bracketPrefix);
123+
return `${leadingTitlePrefix.prefix}: ${normalizedRest}`;
124+
}
125+
126+
const titleWithoutBracketPrefix = removeBracketPrefixToken(leadingTitlePrefix.rest, bracketPrefix);
127+
return `${leadingTitlePrefix.prefix}: ${bracketPrefix}`
128+
+ (titleWithoutBracketPrefix ? ` ${titleWithoutBracketPrefix}` : '');
129+
}
130+
131+
const titleWithoutBracketPrefix = removeBracketPrefixToken(title, bracketPrefix);
132+
return `${bracketPrefix}${titleWithoutBracketPrefix ? ` ${titleWithoutBracketPrefix}` : ''}`;
133+
}
134+
135+
function hasLabel(labels, labelName) {
136+
return labels.some((label) => label.toLowerCase() === labelName.toLowerCase());
137+
}
138+
139+
function getCurrentTitle(context) {
140+
switch (context.eventName) {
141+
case 'issues':
142+
return context.payload.issue.title;
143+
case 'pull_request_target':
144+
return context.payload.pull_request.title;
145+
default:
146+
throw new Error(`Unrecognized eventName: ${context.eventName}`);
147+
}
148+
}
149+
150+
async function updateTitleForAddedLabel({
151+
github,
152+
context,
153+
core,
154+
prefixLabels = DEFAULT_PREFIX_LABELS,
155+
bracketPrefixLabels = DEFAULT_BRACKET_PREFIX_LABELS,
156+
}) {
157+
const labelAdded = context.payload.label?.name;
158+
if (!labelAdded) {
159+
throw new Error('This script must be run from a labeled event.');
160+
}
161+
162+
const currentTitle = getCurrentTitle(context);
163+
let newTitle = null;
164+
165+
const titlePrefix = getMatchingValueByKey(prefixLabels, labelAdded);
166+
if (titlePrefix !== null) {
167+
newTitle = addTitlePrefix(currentTitle, titlePrefix, Object.values(bracketPrefixLabels));
168+
}
169+
170+
const bracketPrefix = getMatchingValueByKey(bracketPrefixLabels, labelAdded);
171+
if (bracketPrefix !== null) {
172+
newTitle = addBracketPrefix(currentTitle, bracketPrefix, Object.values(prefixLabels));
173+
}
174+
175+
if (newTitle === null) {
176+
core.info(`No title prefix configured for label "${labelAdded}".`);
177+
return { updated: false, newTitle: currentTitle };
178+
}
179+
180+
if (newTitle === currentTitle) {
181+
core.info(`Title already includes the prefix for label "${labelAdded}".`);
182+
return { updated: false, newTitle };
183+
}
184+
185+
switch (context.eventName) {
186+
case 'issues':
187+
await github.rest.issues.update({
188+
issue_number: context.issue.number,
189+
owner: context.repo.owner,
190+
repo: context.repo.repo,
191+
title: newTitle,
192+
});
193+
break;
194+
195+
case 'pull_request_target':
196+
await github.rest.pulls.update({
197+
pull_number: context.issue.number,
198+
owner: context.repo.owner,
199+
repo: context.repo.repo,
200+
title: newTitle,
201+
});
202+
break;
203+
204+
default:
205+
throw new Error(`Unrecognized eventName: ${context.eventName}`);
206+
}
207+
208+
return { updated: true, newTitle };
209+
}
210+
211+
async function syncBreakingChangeLabelFromTitle({
212+
github,
213+
context,
214+
core,
215+
labelName = BREAKING_CHANGE_LABEL,
216+
bracketPrefix = BREAKING_PREFIX,
217+
titlePrefixes = Object.values(DEFAULT_PREFIX_LABELS),
218+
}) {
219+
const pullRequest = context.payload.pull_request;
220+
if (!pullRequest) {
221+
throw new Error('This script must be run from a pull_request_target event.');
222+
}
223+
224+
const title = pullRequest.title || '';
225+
if (!hasBracketPrefix(title, bracketPrefix, titlePrefixes)) {
226+
core.info(`Title does not include ${bracketPrefix} in the title prefix.`);
227+
return { added: false };
228+
}
229+
230+
const labels = pullRequest.labels?.map((label) => label.name).filter(Boolean) ?? [];
231+
if (hasLabel(labels, labelName)) {
232+
core.info(`PR already has the "${labelName}" label.`);
233+
return { added: false };
234+
}
235+
236+
await github.rest.issues.addLabels({
237+
issue_number: context.issue.number,
238+
owner: context.repo.owner,
239+
repo: context.repo.repo,
240+
labels: [labelName],
241+
});
242+
243+
return { added: true };
244+
}
245+
246+
module.exports = {
247+
addBracketPrefix,
248+
addTitlePrefix,
249+
hasBracketPrefix,
250+
syncBreakingChangeLabelFromTitle,
251+
updateTitleForAddedLabel,
252+
};

.github/skills/pull-requests/SKILL.md

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,11 @@ no other open PR for the same issue; if there is, explain how this PR differs.
4141
### `### Contribution Checklist`
4242
Check every item that applies. For the breaking-change item:
4343
- Leave **"This is not a breaking change."** checked for the common case.
44-
- If the change **is** breaking, add the `breaking change` label **or** put a
45-
`[BREAKING]` prefix in the title — a workflow keeps the label and the title
46-
prefix in sync automatically (see `.github/workflows/label-title-prefix.yml`
47-
and `.github/workflows/label-breaking-change.yml`).
44+
- If the change **is** breaking, add the `breaking change` label **or** put
45+
`[BREAKING]` in the title prefix, before or after a language prefix such as
46+
`Python:` or `.NET:` — workflows keep the label and the title prefix in sync
47+
automatically (see `.github/workflows/label-title-prefix.yml` and
48+
`.github/workflows/label-pr.yml`).
4849

4950
### Do not
5051
- Do **not** add ad-hoc sections such as "Validation" or "Tests run"; CI/CD and

0 commit comments

Comments
 (0)