Skip to content

Commit 679d648

Browse files
NicolasMassartcursoragentGudahtt
authored
refactor: count PR lines from the GitHub API instead of a git checkout (#273)
* feat: add base branch resolution step in PR line check action This update introduces a new step to resolve the current pull request's base branch dynamically. It ensures that the action uses the live base branch instead of potentially stale data from the webhook payload, improving accuracy in line calculations. * fix: checkout PR head for accurate line counts against live base Co-authored-by: Cursor <cursoragent@cursor.com> * Update .github/actions/pr-line-check/action.yml Co-authored-by: Mark Stacey <markjstacey@gmail.com> * fix: count lines against the resolved live base branch Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: count PR lines from the API instead of a git checkout The pull request files endpoint reports per-file additions and deletions against the pull request's current base, which removes the need to check out the repository, resolve the base branch and probe git history for a merge base. It also makes the count immune to a webhook payload that lags a base retarget, since only the pull request number comes from the event. A pull request touching more than the 3000 files the endpoint lists is labelled size-XL without counting lines, as it is over any limit anyway. Co-authored-by: Cursor <cursoragent@cursor.com> * docs: add changelog entry for pr-line-check API counting Co-authored-by: Cursor <cursoragent@cursor.com> * docs: add changelog entry for pr-line-check API counting Co-authored-by: Cursor <cursoragent@cursor.com> * fix changelog --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Mark Stacey <markjstacey@gmail.com>
1 parent e6bbea1 commit 679d648

2 files changed

Lines changed: 84 additions & 127 deletions

File tree

Lines changed: 80 additions & 127 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,11 @@
11
name: Check PR Lines Changed
2-
description: 'Checks the number of lines changed in a PR and manages size labels accordingly.'
2+
description: 'Counts the lines changed in a PR, applies a size label, and fails when the change exceeds the allowed maximum.'
33

44
inputs:
55
max-lines:
66
description: 'Maximum allowed total lines changed'
77
required: false
88
default: '1000'
9-
base-ref:
10-
description: 'Default base branch to compare against (if not running on a PR)'
11-
required: false
12-
default: 'main'
139
ignore-patterns:
1410
description: 'Regex pattern for files to ignore when calculating changes'
1511
required: false
@@ -34,67 +30,13 @@ inputs:
3430
runs:
3531
using: composite
3632
steps:
37-
- name: Checkout code
38-
uses: actions/checkout@v6
39-
40-
- name: Calculate changed lines
41-
id: line-count
42-
env:
43-
BASE_BRANCH: ${{ github.event.pull_request.base.ref || inputs.base-ref }}
44-
IGNORE_PATTERNS: ${{ inputs.ignore-patterns }}
45-
shell: bash
46-
run: |
47-
set -e
48-
49-
echo "Using base branch: $BASE_BRANCH"
50-
51-
# Instead of a full fetch, perform incremental fetches at increasing depth
52-
# until the merge-base between origin/<BASE_BRANCH> and HEAD is present.
53-
fetch_with_depth() {
54-
local depth=$1
55-
echo "Attempting to fetch with depth $depth..."
56-
git fetch --depth="$depth" origin "$BASE_BRANCH"
57-
}
58-
59-
depths=(1 10 100)
60-
merge_base_found=false
61-
62-
for d in "${depths[@]}"; do
63-
fetch_with_depth "$d"
64-
if git merge-base "origin/$BASE_BRANCH" HEAD > /dev/null 2>&1; then
65-
echo "Merge base found with depth $d."
66-
merge_base_found=true
67-
break
68-
else
69-
echo "Merge base not found with depth $d, increasing depth..."
70-
fi
71-
done
72-
73-
# If we haven't found the merge base with shallow fetches, unshallow the repo.
74-
if [ "$merge_base_found" = false ]; then
75-
echo "Could not find merge base with shallow fetches, fetching full history..."
76-
git fetch --unshallow origin "$BASE_BRANCH" || git fetch origin "$BASE_BRANCH"
77-
fi
78-
79-
# Calculate additions and deletions across all changes between the base and HEAD,
80-
# filtering out files matching the ignore pattern.
81-
additions=$(git diff "origin/$BASE_BRANCH"...HEAD --numstat | grep -Ev "$IGNORE_PATTERNS" | awk '{add += $1} END {print add+0}')
82-
deletions=$(git diff "origin/$BASE_BRANCH"...HEAD --numstat | grep -Ev "$IGNORE_PATTERNS" | awk '{del += $2} END {print del+0}')
83-
total=$((additions + deletions))
84-
85-
echo "Additions: $additions, Deletions: $deletions, Total: $total"
86-
{
87-
echo "lines-changed=$total"
88-
echo "additions=$additions"
89-
echo "deletions=$deletions"
90-
} >> "$GITHUB_OUTPUT"
91-
92-
- name: Check line count limit
33+
# The API computes the file list against the PR's current base, so no
34+
# checkout, base-branch resolution or history fetching is needed, and a
35+
# webhook payload that lags a base retarget cannot skew the count.
36+
- name: Count changed lines and apply size label
9337
uses: actions/github-script@v9
9438
env:
95-
LINES_CHANGED: ${{ steps.line-count.outputs.lines-changed }}
96-
ADDITIONS: ${{ steps.line-count.outputs.additions }}
97-
DELETIONS: ${{ steps.line-count.outputs.deletions }}
39+
IGNORE_PATTERNS: ${{ inputs.ignore-patterns }}
9840
MAX_LINES: ${{ inputs.max-lines }}
9941
XS_MAX_SIZE: ${{ inputs.xs-max-size }}
10042
S_MAX_SIZE: ${{ inputs.s-max-size }}
@@ -103,101 +45,112 @@ runs:
10345
with:
10446
script: |
10547
const {
106-
LINES_CHANGED,
107-
ADDITIONS,
108-
DELETIONS,
48+
IGNORE_PATTERNS,
10949
MAX_LINES,
11050
XS_MAX_SIZE,
11151
S_MAX_SIZE,
11252
M_MAX_SIZE,
11353
L_MAX_SIZE,
11454
} = process.env;
11555
116-
const total = parseInt(LINES_CHANGED, 10) || 0;
117-
const additions = parseInt(ADDITIONS, 10) || 0;
118-
const deletions = parseInt(DELETIONS, 10) || 0;
56+
if (!context.payload.pull_request) {
57+
core.setFailed('This action must run on a pull_request event.');
58+
return;
59+
}
60+
61+
const { owner, repo } = context.repo;
62+
const pullNumber = context.payload.pull_request.number;
63+
64+
const { data: pr } = await github.rest.pulls.get({
65+
owner,
66+
repo,
67+
pull_number: pullNumber,
68+
});
69+
70+
const files = await github.paginate(github.rest.pulls.listFiles, {
71+
owner,
72+
repo,
73+
pull_number: pullNumber,
74+
per_page: 100,
75+
});
76+
77+
// The API lists at most 3000 files. A PR touching more than that is
78+
// past any configured limit, so it goes straight to size-XL rather
79+
// than being counted from a truncated file list.
80+
const isCountable = files.length >= pr.changed_files;
81+
82+
const ignored = new RegExp(IGNORE_PATTERNS);
83+
const counted = files.filter((file) => !ignored.test(file.filename));
84+
const additions = counted.reduce((sum, file) => sum + file.additions, 0);
85+
const deletions = counted.reduce((sum, file) => sum + file.deletions, 0);
86+
const total = additions + deletions;
11987
120-
// Thresholds from inputs with fallback to defaults
12188
const maxLines = parseInt(MAX_LINES, 10) || 1000;
122-
const xsMaxSize = parseInt(XS_MAX_SIZE, 10) || 10;
123-
const sMaxSize = parseInt(S_MAX_SIZE, 10) || 100;
124-
const mMaxSize = parseInt(M_MAX_SIZE, 10) || 500;
125-
const lMaxSize = parseInt(L_MAX_SIZE, 10) || 1000;
12689
127-
// Print summary
128-
console.log('Summary:');
129-
console.log(` - Additions: ${additions}`);
130-
console.log(` - Deletions: ${deletions}`);
131-
console.log(` - Total: ${total}`);
132-
console.log(` - Limit: ${maxLines}`);
90+
const sizeThresholds = [
91+
['size-XS', parseInt(XS_MAX_SIZE, 10) || 10],
92+
['size-S', parseInt(S_MAX_SIZE, 10) || 100],
93+
['size-M', parseInt(M_MAX_SIZE, 10) || 500],
94+
['size-L', parseInt(L_MAX_SIZE, 10) || 1000],
95+
];
96+
const sizeLabel = isCountable
97+
? (sizeThresholds.find(([, threshold]) => total <= threshold)?.[0] ?? 'size-XL')
98+
: 'size-XL';
13399
134-
// Determine size label based on configured criteria
135-
let sizeLabel = '';
136-
if (total <= xsMaxSize) {
137-
sizeLabel = 'size-XS';
138-
} else if (total <= sMaxSize) {
139-
sizeLabel = 'size-S';
140-
} else if (total <= mMaxSize) {
141-
sizeLabel = 'size-M';
142-
} else if (total <= lMaxSize) {
143-
sizeLabel = 'size-L';
100+
console.log('Summary:');
101+
console.log(` - Base branch: ${pr.base.ref}`);
102+
if (isCountable) {
103+
console.log(` - Additions: ${additions}`);
104+
console.log(` - Deletions: ${deletions}`);
105+
console.log(` - Total: ${total}`);
144106
} else {
145-
sizeLabel = 'size-XL';
107+
console.log(` - Changed files: ${pr.changed_files}, more than the API lists`);
146108
}
147-
109+
console.log(` - Limit: ${maxLines}`);
148110
console.log(` - Size category: ${sizeLabel}`);
149111
150-
// Manage PR labels
151-
const owner = context.repo.owner;
152-
const repo = context.repo.repo;
153-
const issue_number = context.payload.pull_request.number;
112+
const allSizeLabels = ['size-XS', 'size-S', 'size-M', 'size-L', 'size-XL'];
154113
155114
try {
156-
const existingSizeLabels = ['size-XS', 'size-S', 'size-M', 'size-L', 'size-XL'];
157-
158-
// Get current labels
159-
const currentLabels = await github.rest.issues.listLabelsOnIssue({
115+
const { data: labels } = await github.rest.issues.listLabelsOnIssue({
160116
owner,
161117
repo,
162-
issue_number
118+
issue_number: pullNumber,
163119
});
164120
165-
const currentLabelNames = currentLabels.data.map(l => l.name);
166-
167-
// Build new label set: keep non-size labels and add the new size label
168-
const newLabels = currentLabelNames
169-
.filter(name => !existingSizeLabels.includes(name)) // Remove all size labels
170-
.concat(sizeLabel); // Add the correct size label
121+
const currentNames = labels.map((label) => label.name);
122+
const currentSizeLabels = currentNames.filter((name) =>
123+
allSizeLabels.includes(name),
124+
);
171125
172-
// Check if labels need updating
173-
const currentSizeLabel = currentLabelNames.find(name => existingSizeLabels.includes(name));
174-
if (currentSizeLabel === sizeLabel && currentLabelNames.length === newLabels.length) {
126+
if (currentSizeLabels.length === 1 && currentSizeLabels[0] === sizeLabel) {
175127
console.log(`✅ Correct label '${sizeLabel}' already present, no changes needed`);
176128
} else {
177-
// Update all labels in a single API call
178129
await github.rest.issues.setLabels({
179130
owner,
180131
repo,
181-
issue_number,
182-
labels: newLabels
132+
issue_number: pullNumber,
133+
labels: currentNames
134+
.filter((name) => !allSizeLabels.includes(name))
135+
.concat(sizeLabel),
183136
});
184137
185-
if (currentSizeLabel && currentSizeLabel !== sizeLabel) {
186-
console.log(` - Replaced '${currentSizeLabel}' with '${sizeLabel}'`);
187-
} else if (!currentSizeLabel) {
188-
console.log(`✅ Added '${sizeLabel}' label to PR #${issue_number}`);
189-
} else {
190-
console.log(`✅ Updated labels for PR #${issue_number}`);
191-
}
138+
console.log(`✅ Set '${sizeLabel}' on PR #${pullNumber}`);
192139
}
193140
} catch (error) {
194-
console.log(`⚠️ Could not manage labels: ${error.message}`);
141+
console.log(`⚠️ Could not update labels: ${error.message}`);
142+
}
143+
144+
if (!isCountable) {
145+
core.setFailed(
146+
`PR touches ${pr.changed_files} files, too many to count lines for, so it is over the limit of ${maxLines}.`,
147+
);
148+
return;
195149
}
196150
197-
// Check if exceeds limit
198151
if (total > maxLines) {
199-
console.log(`❌ Error: Total changed lines (${total}) exceed the limit of ${maxLines}.`);
200-
process.exit(1);
201-
} else {
202-
console.log(`✅ Success: Total changed lines (${total}) are within the limit of ${maxLines}.`);
152+
core.setFailed(`Total changed lines (${total}) exceed the limit of ${maxLines}.`);
153+
return;
203154
}
155+
156+
console.log(`✅ Success: Total changed lines (${total}) are within the limit of ${maxLines}.`);

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Fixed
11+
12+
- Count changed lines in `pr-line-check` from the pull request files API, so the count always reflects the pull request's current base branch ([#273](https://github.com/MetaMask/github-tools/pull/273))
13+
1014
## [1.18.1]
1115

1216
### Fixed

0 commit comments

Comments
 (0)