Skip to content

Commit cd7f2dd

Browse files
committed
labels: fix lib/src bucket for src/ changes
This makes the `lib / src` bucket label also include files changes in `src/`. Previously it only was exclusive for `lib/` files.
1 parent 932b06b commit cd7f2dd

File tree

2 files changed

+48
-15
lines changed

2 files changed

+48
-15
lines changed

lib/node-labels.js

+15-15
Original file line numberDiff line numberDiff line change
@@ -130,19 +130,19 @@ const exclusiveLabelsMap = new Map([
130130
[/^benchmark\//, 'benchmark']
131131
])
132132

133-
function resolveLabels (filepathsChanged, baseBranch, limitLib = true) {
133+
function resolveLabels (filepathsChanged, baseBranch, limitLabels = true) {
134134
const exclusiveLabels = matchExclusiveSubSystem(filepathsChanged)
135135

136136
if (typeof baseBranch !== 'string') {
137137
if (typeof baseBranch === 'boolean') {
138-
limitLib = baseBranch
138+
limitLabels = baseBranch
139139
}
140140
baseBranch = ''
141141
}
142142

143143
const labels = (exclusiveLabels.length > 0)
144144
? exclusiveLabels
145-
: matchAllSubSystem(filepathsChanged, limitLib)
145+
: matchAllSubSystem(filepathsChanged, limitLabels)
146146

147147
// Add version labels if PR is made against a version branch
148148
const m = /^(v\d+\.(?:\d+|x))(?:-staging|$)/.exec(baseBranch)
@@ -195,13 +195,13 @@ function matchExclusiveSubSystem (filepathsChanged) {
195195
return isExclusive ? labels : []
196196
}
197197

198-
function matchAllSubSystem (filepathsChanged, limitLib) {
198+
function matchAllSubSystem (filepathsChanged, limitLabels) {
199199
return matchSubSystemsByRegex(
200-
subSystemLabelsMap, filepathsChanged, limitLib)
200+
subSystemLabelsMap, filepathsChanged, limitLabels)
201201
}
202202

203-
function matchSubSystemsByRegex (rxLabelsMap, filepathsChanged, limitLib) {
204-
const jsLabelCount = []
203+
function matchSubSystemsByRegex (rxLabelsMap, filepathsChanged, limitLabels) {
204+
const labelCount = []
205205
// by putting matched labels into a map, we avoid duplicate labels
206206
const labelsMap = filepathsChanged.reduce((map, filepath) => {
207207
const mappedSubSystems = mappedSubSystemsForFile(rxLabelsMap, filepath)
@@ -213,16 +213,17 @@ function matchSubSystemsByRegex (rxLabelsMap, filepathsChanged, limitLib) {
213213

214214
for (var i = 0; i < mappedSubSystems.length; ++i) {
215215
const mappedSubSystem = mappedSubSystems[i]
216-
if (limitLib && hasJsSubsystemChanges(filepathsChanged, mappedSubSystem)) {
217-
if (jsLabelCount.length >= 4) {
218-
for (const jsLabel of jsLabelCount) {
219-
delete map[jsLabel]
216+
if (limitLabels && hasLibOrSrcChanges(filepathsChanged)) {
217+
if (labelCount.length >= 4) {
218+
for (const label of labelCount) {
219+
// don't delete the c++ label as we always want that if it has matched
220+
if (label !== 'c++') delete map[label]
220221
}
221222
map['lib / src'] = true
222223
// short-circuit
223224
return map
224225
} else {
225-
jsLabelCount.push(mappedSubSystem)
226+
labelCount.push(mappedSubSystem)
226227
}
227228
}
228229

@@ -235,9 +236,8 @@ function matchSubSystemsByRegex (rxLabelsMap, filepathsChanged, limitLib) {
235236
return Object.keys(labelsMap)
236237
}
237238

238-
function hasJsSubsystemChanges (filepathsChanged, mappedSubSystem) {
239-
const hasLibChanges = filepathsChanged.some((filepath) => filepath.startsWith('lib/'))
240-
return hasLibChanges && jsSubsystemList.includes(mappedSubSystem)
239+
function hasLibOrSrcChanges (filepathsChanged) {
240+
return filepathsChanged.some((filepath) => filepath.startsWith('lib/') || filepath.startsWith('src/'))
241241
}
242242

243243
function mappedSubSystemsForFile (labelsMap, filepath) {

test/unit/node-labels.test.js

+33
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,39 @@ tap.test('label: "lib / src" when 5 or more JS sub-systems have been changed', (
235235
t.end()
236236
})
237237

238+
// https://github.com/nodejs/node/pull/12366 should have been labelled "lib / src"
239+
// https://github.com/nodejs/github-bot/issues/137
240+
tap.test('label: "lib / src" when 5 or more native files have been changed', (t) => {
241+
const labels = nodeLabels.resolveLabels([
242+
'node.gyp',
243+
'src/cares_wrap.cc',
244+
'src/fs_event_wrap.cc',
245+
'src/node.cc',
246+
'src/node_api.cc',
247+
'src/node_buffer.cc',
248+
'src/node_config.cc',
249+
'src/node_constants.cc',
250+
'src/node_contextify.cc',
251+
'src/node_file.cc',
252+
'src/node_file.h',
253+
'src/node_http_parser.cc',
254+
'src/node_http_parser.h',
255+
'src/node_i18n.cc',
256+
'src/node_revert.cc',
257+
'src/node_serdes.cc',
258+
'src/node_zlib.cc',
259+
'src/process_wrap.cc',
260+
'src/signal_wrap.cc',
261+
'src/string_bytes.cc',
262+
'src/timer_wrap.cc',
263+
'src/uv.cc'
264+
])
265+
266+
t.same(labels, ['c++', 'lib / src'])
267+
268+
t.end()
269+
})
270+
238271
// https://github.com/nodejs/node/pull/7488 wrongfully labelled with "lib / src"
239272
tap.test('label: not "lib / src" when only deps have been changed', (t) => {
240273
const labels = nodeLabels.resolveLabels([

0 commit comments

Comments
 (0)