From daacca18313062992b265818e1f55fd37d6ede68 Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Tue, 13 Dec 2016 13:47:58 -0500 Subject: [PATCH 1/2] attempt-backport: unlabel previous bot labels --- lib/node-repo.js | 35 +++++++++++++++++++++++++++++++++++ scripts/attempt-backport.js | 37 ++++++++++++++++++++++++++++++++----- 2 files changed, 67 insertions(+), 5 deletions(-) diff --git a/lib/node-repo.js b/lib/node-repo.js index 23063960..6b98ca07 100644 --- a/lib/node-repo.js +++ b/lib/node-repo.js @@ -137,6 +137,40 @@ function fetchLabelPages (options, startPageNum, cb) { }) } +function getBotPrLabels (options, cb) { + githubClient.issues.getEvents({ + owner: options.owner, + repo: options.repo, + page: 1, + per_page: 100, // we probably won't hit this + issue_number: options.prId + }, (err, events) => { + if (err) { + return cb(err) + } + + const ourLabels = [] + + for (const event of events) { + if (event.event === 'unlabeled') { + const index = ourLabels.indexOf(event.label.name) + if (index === -1) continue + + ourLabels.splice(index, 1) + } else if (event.event === 'labeled') { + const index = ourLabels.indexOf(event.label.name) + if (index > -1) continue + + if (event.actor.login === 'nodejs-github-bot') { + ourLabels.push(event.label.name) + } + } + } + + cb(null, ourLabels) + }) +} + function stringsInCommon (arr1, arr2) { const loweredArr2 = arr2.map((str) => str.toLowerCase()) // we want the original string cases in arr1, therefore we don't lowercase them @@ -144,6 +178,7 @@ function stringsInCommon (arr1, arr2) { return arr1.filter((str) => loweredArr2.indexOf(str.toLowerCase()) !== -1) } +exports.getBotPrLabels = getBotPrLabels exports.removeLabelFromPR = removeLabelFromPR exports.updatePrWithLabels = updatePrWithLabels exports.resolveLabelsThenUpdatePr = deferredResolveLabelsThenUpdatePr diff --git a/scripts/attempt-backport.js b/scripts/attempt-backport.js index f61a4108..97082e0d 100644 --- a/scripts/attempt-backport.js +++ b/scripts/attempt-backport.js @@ -5,7 +5,8 @@ const debug = require('debug')('attempt-backport') const request = require('request') const node_repo = require('../lib/node-repo') const updatePrWithLabels = node_repo.updatePrWithLabels -// const removeLabelFromPR = node_repo.removeLabelFromPR +const removeLabelFromPR = node_repo.removeLabelFromPR +const getBotPrLabels = node_repo.getBotPrLabels const enabledRepos = ['node'] const nodeVersions = [ @@ -129,7 +130,23 @@ function attemptBackport (options, version, isLTS, cb) { const _cb = cb setImmediate(() => { options.logger.debug(`backport to ${version} failed`) - if (!isLTS) updatePrWithLabels(options, [`dont-land-on-v${version}.x`]) + + if (!isLTS) { + updatePrWithLabels(options, [`dont-land-on-v${version}.x`]) + } else { + getBotPrLabels(options, (err, ourLabels) => { + if (err) { + options.logger.error(err, 'Error fetching existing bot labels') + return + } + + const label = `lts-watch-v${version}.x` + if (!ourLabels.includes(label)) return + + removeLabelFromPR(options, label) + }) + } + setImmediate(() => { inProgress = false _cb() @@ -207,11 +224,21 @@ function attemptBackport (options, version, isLTS, cb) { // Success! if (isLTS) { updatePrWithLabels(options, [`lts-watch-v${version}.x`]) - }// else { + } else { // TODO(Fishrock123): Re-enable this, but do a check first // to make sure the label was set by the bot only. - // removeLabelFromPR(options, `dont-land-on-v${version}.x`) - // } + getBotPrLabels(options, (err, ourLabels) => { + if (err) { + options.logger.error(err, 'Error fetching existing bot labels') + return + } + + const label = `dont-land-on-v${version}.x` + if (!ourLabels.includes(label)) return + + removeLabelFromPR(options, label) + }) + } setImmediate(() => { options.logger.debug(`backport to v${version} successful`) From a13ed9a17317841b8ab2a9da052d11316e1ed883 Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Tue, 13 Dec 2016 13:52:48 -0500 Subject: [PATCH 2/2] attempt-backport: check existing labels before updating --- lib/node-repo.js | 2 +- scripts/attempt-backport.js | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/node-repo.js b/lib/node-repo.js index 6b98ca07..e133bec0 100644 --- a/lib/node-repo.js +++ b/lib/node-repo.js @@ -180,7 +180,7 @@ function stringsInCommon (arr1, arr2) { exports.getBotPrLabels = getBotPrLabels exports.removeLabelFromPR = removeLabelFromPR -exports.updatePrWithLabels = updatePrWithLabels +exports.fetchExistingThenUpdatePr = fetchExistingThenUpdatePr exports.resolveLabelsThenUpdatePr = deferredResolveLabelsThenUpdatePr // exposed for testability diff --git a/scripts/attempt-backport.js b/scripts/attempt-backport.js index 97082e0d..5a0ccb89 100644 --- a/scripts/attempt-backport.js +++ b/scripts/attempt-backport.js @@ -4,7 +4,7 @@ const child_process = require('child_process') const debug = require('debug')('attempt-backport') const request = require('request') const node_repo = require('../lib/node-repo') -const updatePrWithLabels = node_repo.updatePrWithLabels +const fetchExistingThenUpdatePr = node_repo.fetchExistingThenUpdatePr const removeLabelFromPR = node_repo.removeLabelFromPR const getBotPrLabels = node_repo.getBotPrLabels @@ -132,7 +132,7 @@ function attemptBackport (options, version, isLTS, cb) { options.logger.debug(`backport to ${version} failed`) if (!isLTS) { - updatePrWithLabels(options, [`dont-land-on-v${version}.x`]) + fetchExistingThenUpdatePr(options, [`dont-land-on-v${version}.x`]) } else { getBotPrLabels(options, (err, ourLabels) => { if (err) { @@ -223,7 +223,7 @@ function attemptBackport (options, version, isLTS, cb) { const cp = wrapCP('git', ['am'], { stdio: 'pipe' }, function done () { // Success! if (isLTS) { - updatePrWithLabels(options, [`lts-watch-v${version}.x`]) + fetchExistingThenUpdatePr(options, [`lts-watch-v${version}.x`]) } else { // TODO(Fishrock123): Re-enable this, but do a check first // to make sure the label was set by the bot only.