Skip to content
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

Jump to bottom

fix: Limit number of labels added to 100 #497

Merged
merged 23 commits into from
Jun 21, 2023
Merged

fix: Limit number of labels added to 100 #497

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
756f0bd
push to excess labels to avoid reaching the limit
markmssd Feb 2, 2023
e4e6956
build dist
markmssd Feb 2, 2023
7429fbb
never set more than 100 labels
markmssd Mar 21, 2023
e173de7
use splice instead of set
markmssd Mar 21, 2023
56b1a22
Merge branch 'main' into fix/max-100-labels
markmssd Apr 18, 2023
f90e3c3
Merge branch 'main' into fix/max-100-labels
markmssd Jun 8, 2023
0e55d23
ignore IDE folders
markmssd Jun 8, 2023
4beee0f
install @octokit/plugin-retry
markmssd Jun 8, 2023
267e6ba
always setLabels
markmssd Jun 8, 2023
b35f25a
fix indentations
markmssd Jun 8, 2023
5baeff5
fix specs
markmssd Jun 8, 2023
d65a48d
add spec for excess labels
markmssd Jun 8, 2023
4dd2b81
prettier
markmssd Jun 12, 2023
2f9caa0
licensed cache
markmssd Jun 12, 2023
112b189
revert to !!core.getInput('sync-labels')
markmssd Jun 13, 2023
3d7acab
better warning for exceeded labels
markmssd Jun 13, 2023
92a11d8
keep manually-added labels
markmssd Jun 13, 2023
ffc8ca7
nest the dedupe logic
markmssd Jun 13, 2023
3c2f134
rename `removeLabel` to `removeLabelFromList` to avoid confusion
markmssd Jun 15, 2023
66f07b8
Merge branch 'main' into fix/max-100-labels
unrealclouder Jun 19, 2023
287ce90
use Sets, and issue a call only if labels have actually changed
unrealclouder Jun 20, 2023
05a4b4b
remove IDE config folders from gitignore
unrealclouder Jun 20, 2023
c9ee1b4
remove obsolete duplucation check
unrealclouder Jun 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 14 additions & 3 deletions dist/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ const core = __importStar(__nccwpck_require__(2186));
const github = __importStar(__nccwpck_require__(5438));
const yaml = __importStar(__nccwpck_require__(1917));
const minimatch_1 = __nccwpck_require__(3973);
// Github Issues cannot have more than 100 labels
const GITHUB_MAX_LABELS = 100;
function run() {
return __awaiter(this, void 0, void 0, function* () {
try {
Expand All @@ -66,20 +68,29 @@ function run() {
const labelGlobs = yield getLabelGlobs(client, configPath);
const labels = [];
const labelsToRemove = [];
const excessLabels = [];
for (const [label, globs] of labelGlobs.entries()) {
core.debug(`processing ${label}`);
if (checkGlobs(changedFiles, globs)) {
labels.push(label);
if (labels.length >= GITHUB_MAX_LABELS) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this neglects the number of labels that are already present. Say the PR has 50 labels, and we come up with 100 that we want to apply, but only 25 of those are already present. The correct result would be to add 50 new ones, 25 remain the same, and 25 would be excess. But the current code reports 100 labels with no excess.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I've moved the addLabels to happen AFTER the removeLabels (which happens with syncLabels enabled only) to mitigate this issue. But maybe there's a better way still, I'll check again. Thanks!

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me, an easy solution seems to be first getting the labels already on the issue with listLabelsOnIssue. Then, you have all the information needed to ensure the total count remains less than 100 while tracking accurately all additions/removals that you want to do.

excessLabels.push(label);
}
else {
labels.push(label);
}
}
else if (pullRequest.labels.find(l => l.name === label)) {
labelsToRemove.push(label);
}
}
if (syncLabels && labelsToRemove.length) {
yield removeLabels(client, prNumber, labelsToRemove);
}
if (labels.length > 0) {
yield addLabels(client, prNumber, labels);
}
if (syncLabels && labelsToRemove.length) {
yield removeLabels(client, prNumber, labelsToRemove);
if (excessLabels.length > 0) {
core.warning(`failed to add excess labels ${excessLabels.join(', ')}`);
}
}
catch (error) {
Expand Down
18 changes: 15 additions & 3 deletions src/labeler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ interface MatchConfig {
type StringOrMatchConfig = string | MatchConfig;
type ClientType = ReturnType<typeof github.getOctokit>;

// Github Issues cannot have more than 100 labels
const GITHUB_MAX_LABELS = 100;

export async function run() {
try {
const token = core.getInput('repo-token', {required: true});
Expand Down Expand Up @@ -40,21 +43,30 @@ export async function run() {

const labels: string[] = [];
const labelsToRemove: string[] = [];
const excessLabels: string[] = [];
for (const [label, globs] of labelGlobs.entries()) {
core.debug(`processing ${label}`);
if (checkGlobs(changedFiles, globs)) {
labels.push(label);
if (labels.length >= GITHUB_MAX_LABELS) {
excessLabels.push(label);
} else {
labels.push(label);
}
} else if (pullRequest.labels.find(l => l.name === label)) {
labelsToRemove.push(label);
}
}

if (syncLabels && labelsToRemove.length) {
await removeLabels(client, prNumber, labelsToRemove);
}

if (labels.length > 0) {
await addLabels(client, prNumber, labels);
}

if (syncLabels && labelsToRemove.length) {
await removeLabels(client, prNumber, labelsToRemove);
if (excessLabels.length > 0) {
core.warning(`failed to add excess labels ${excessLabels.join(', ')}`);
markmssd marked this conversation as resolved.
Show resolved Hide resolved
}
} catch (error: any) {
core.error(error);
Expand Down