From 04e20dce8eaffbfdaeb919b3595c19e67af5ef17 Mon Sep 17 00:00:00 2001 From: Rhys Arkins Date: Fri, 7 Jul 2017 06:25:38 +0200 Subject: [PATCH] feat: Prune all orphan branches (#461) Schedule logic has been refactored to enable the pruning of all orphan branches. Now, schedules aren't checked at package-time, instead package upgrades are queued regardless of schedule. At branch time, it is checked whether the *branch* is scheduled and then the branch is not created/updated if it's off-schedule. This enables the repository worker to know all possible branches and hence be able to determine which remaining branches in the repository are "orphans" to be deleted. Closes #428, Closes #426 --- lib/workers/branch/index.js | 28 ++++++------ lib/workers/{package => branch}/schedule.js | 0 lib/workers/package-file/index.js | 26 ++++++----- lib/workers/package/index.js | 11 ----- lib/workers/repository/cleanup.js | 45 +++---------------- lib/workers/repository/index.js | 8 ++-- lib/workers/repository/onboarding.js | 22 ++++----- lib/workers/repository/upgrades.js | 15 ++++++- test/workers/branch/index.spec.js | 8 ++++ .../{package => branch}/schedule.spec.js | 4 +- test/workers/package-file/index.spec.js | 17 +++---- test/workers/package/index.spec.js | 12 +---- .../__snapshots__/onboarding.spec.js.snap | 38 ++++++++++++++++ .../__snapshots__/upgrades.spec.js.snap | 29 ++++++++++++ test/workers/repository/cleanup.spec.js | 45 +------------------ test/workers/repository/onboarding.spec.js | 18 ++++++++ test/workers/repository/upgrades.spec.js | 22 +++++++++ 17 files changed, 188 insertions(+), 160 deletions(-) rename lib/workers/{package => branch}/schedule.js (100%) rename test/workers/{package => branch}/schedule.spec.js (97%) diff --git a/lib/workers/branch/index.js b/lib/workers/branch/index.js index c6fcf2d42f..da900af45b 100644 --- a/lib/workers/branch/index.js +++ b/lib/workers/branch/index.js @@ -2,6 +2,7 @@ const handlebars = require('handlebars'); const packageJsonHelper = require('./package-json'); const npm = require('./npm'); const yarn = require('./yarn'); +const schedule = require('./schedule'); const prWorker = require('../pr'); let logger = require('../../logger'); @@ -191,23 +192,22 @@ async function ensureBranch(config) { } async function processBranchUpgrades(branchUpgrades, errors, warnings) { + logger = branchUpgrades.logger || logger; logger.trace({ config: branchUpgrades }, 'processBranchUpgrades'); - let createdBranch; const config = Object.assign({}, branchUpgrades); - // Delete the semanticPrefix for this branch if feature is not enabled - if (config.semanticCommits) { - logger.debug('Branch has semantic commits enabled'); - } else { - logger.debug('Branch has semantic commits disabled'); - delete config.semanticPrefix; + // Check schedule + if ( + config.schedule && + config.schedule.length && + schedule.isScheduledNow(config) === false + ) { + logger.info('Skipping branch as it is not scheduled'); + return; } - // Use templates to generate strings - const branchName = handlebars.compile(config.branchName)(config); - const prTitle = handlebars.compile(config.prTitle)(config); logger = logger.child({ repository: config.repository, - branch: branchName, + branch: config.branchName, }); config.logger = logger; @@ -218,14 +218,13 @@ async function processBranchUpgrades(branchUpgrades, errors, warnings) { if ( // Groups and lock file maintenance should set this to true config.recreateClosed === false && - (await config.api.checkForClosedPr(branchName, prTitle)) + (await config.api.checkForClosedPr(config.branchName, config.prTitle)) ) { logger.info(`Skipping branch as matching closed PR already existed`); - return null; + return; } const branchCreated = await module.exports.ensureBranch(config); if (branchCreated) { - createdBranch = branchName; const pr = await prWorker.ensurePr( config.upgrades, logger, @@ -241,5 +240,4 @@ async function processBranchUpgrades(branchUpgrades, errors, warnings) { logger.debug(JSON.stringify(err)); // Don't throw here - we don't want to stop the other renovations } - return createdBranch; } diff --git a/lib/workers/package/schedule.js b/lib/workers/branch/schedule.js similarity index 100% rename from lib/workers/package/schedule.js rename to lib/workers/branch/schedule.js diff --git a/lib/workers/package-file/index.js b/lib/workers/package-file/index.js index adddc6b59d..baf473fe25 100644 --- a/lib/workers/package-file/index.js +++ b/lib/workers/package-file/index.js @@ -1,6 +1,6 @@ +const path = require('path'); const configParser = require('../../config'); const depTypeWorker = require('../dep-type'); -const schedule = require('../package/schedule'); let logger = require('../../logger'); @@ -55,16 +55,22 @@ async function findUpgrades(packageFileConfig) { ); } - // Maintain lock files - const lockFileMaintenanceConf = Object.assign( - {}, - config, - config.lockFileMaintenance + // Detect if a yarn.lock file is in use + const yarnLockFileName = path.join( + path.dirname(config.packageFile), + 'yarn.lock' ); - if (lockFileMaintenanceConf.enabled) { - logger.debug('lockFileMaintenance enabled'); - lockFileMaintenanceConf.type = 'lockFileMaintenance'; - if (schedule.isScheduledNow(lockFileMaintenanceConf)) { + if (await config.api.getFileContent(yarnLockFileName)) { + config.hasYarnLock = true; + // Maintain lock files + const lockFileMaintenanceConf = Object.assign( + {}, + config, + config.lockFileMaintenance + ); + if (lockFileMaintenanceConf.enabled) { + logger.debug('lockFileMaintenance enabled'); + lockFileMaintenanceConf.type = 'lockFileMaintenance'; logger.debug(`lock config=${JSON.stringify(lockFileMaintenanceConf)}`); upgrades.push(lockFileMaintenanceConf); } diff --git a/lib/workers/package/index.js b/lib/workers/package/index.js index 4411d3ed4a..3ab2679844 100644 --- a/lib/workers/package/index.js +++ b/lib/workers/package/index.js @@ -1,6 +1,5 @@ const npmApi = require('../../api/npm'); const versions = require('./versions'); -const schedule = require('./schedule'); const configParser = require('../../config'); let logger = require('../../logger'); @@ -16,16 +15,6 @@ async function findUpgrades(config) { logger.debug('package is disabled'); return []; } - // Check schedule - if ( - config.repoIsOnboarded && - config.schedule && - config.schedule.length && - schedule.isScheduledNow(config) === false - ) { - logger.debug('Skipping package as it is not scheduled'); - return []; - } let results = []; const npmDep = await npmApi.getDependency(config.depName, logger); if (npmDep) { diff --git a/lib/workers/repository/cleanup.js b/lib/workers/repository/cleanup.js index bd78486b4e..5771c6b1f5 100644 --- a/lib/workers/repository/cleanup.js +++ b/lib/workers/repository/cleanup.js @@ -2,64 +2,29 @@ module.exports = { pruneStaleBranches, }; -async function pruneStaleBranches(config, branchUpgradeNames) { +async function pruneStaleBranches(config, branchList) { const logger = config.logger; logger.debug('Removing any stale branches'); logger.trace( { config }, - `pruneStaleBranches:\n${JSON.stringify(branchUpgradeNames)}` + `pruneStaleBranches:\n${JSON.stringify(branchList)}` ); if (config.platform !== 'github') { logger.debug('Platform is not GitHub - returning'); return; } - if (branchUpgradeNames.length === 0) { - logger.debug('No branchUpgradeNames - returning'); - return; - } const renovateBranches = await config.api.getAllRenovateBranches(); logger.debug(`renovateBranches=${renovateBranches}`); const remainingBranches = renovateBranches.filter( - branch => branchUpgradeNames.indexOf(branch) === -1 + branch => branchList.indexOf(branch) === -1 ); logger.debug(`remainingBranches=${remainingBranches}`); if (remainingBranches.length === 0) { logger.debug('No branches to clean up'); return; } - const allPrs = await config.api.getAllPrs(); for (const branchName of remainingBranches) { - logger.debug({ branch: branchName }, `Checking orphan branch for deletion`); - // Default to deleting the branch if we don't find a PR - let deleteBranch = true; - let foundPr = false; - for (const pr of allPrs) { - if (pr.state === 'open' && pr.branchName === branchName) { - // We have a matching PR - foundPr = true; - logger.debug({ branch: branchName }, `Found matching PR#${pr.number}`); - const prDetails = config.api.getPr(pr.number); - if (prDetails.mergeable) { - // Don't delete the branch if we found a mergeable PR - logger.debug( - { branch: branchName }, - 'PR is mergeable, so do not delete' - ); - deleteBranch = false; - } else { - logger.debug( - { branch: branchName }, - 'PR is not mergeable, we will delete' - ); - } - } - } - if (deleteBranch) { - if (!foundPr) { - logger.debug({ branch: branchName }, 'Orphan branch has no PR'); - } - logger.info({ branch: branchName }, `Deleting orphan branch`); - await config.api.deleteBranch(branchName); - } + logger.debug({ branch: branchName }, `Deleting orphan branch`); + await config.api.deleteBranch(branchName); } } diff --git a/lib/workers/repository/index.js b/lib/workers/repository/index.js index 10d73db1df..983a634d07 100644 --- a/lib/workers/repository/index.js +++ b/lib/workers/repository/index.js @@ -78,17 +78,15 @@ async function renovateRepository(packageFileConfig) { config.logger.debug(`Updating ${branchUpgrades.length} branch(es)`); config.logger.trace({ config: branchUpgrades }, 'branchUpgrades'); if (config.repoIsOnboarded) { - const branchList = []; for (const branchUpgrade of branchUpgrades) { - const createdBranchName = await branchWorker.processBranchUpgrades( + await branchWorker.processBranchUpgrades( branchUpgrade, config.errors, config.warnings ); - if (createdBranchName) { - branchList.push(createdBranchName); - } } + const branchList = branchUpgrades.map(upgrade => upgrade.branchName); + config.logger.debug(`branchList=${branchList}`); await cleanup.pruneStaleBranches(config, branchList); } else { await onboarding.ensurePr(config, branchUpgrades); diff --git a/lib/workers/repository/onboarding.js b/lib/workers/repository/onboarding.js index 28a483de06..96017a1347 100644 --- a/lib/workers/repository/onboarding.js +++ b/lib/workers/repository/onboarding.js @@ -1,4 +1,3 @@ -const handlebars = require('handlebars'); const stringify = require('json-stringify-pretty-compact'); const configParser = require('../../config'); @@ -95,21 +94,24 @@ With your current configuration, renovate will initially create the following Pu | ------ | `; branchUpgrades.forEach(branch => { - const branchName = handlebars.compile(branch.branchName)(branch); - const prTitle = handlebars.compile(branch.prTitle)(branch); - prDesc += `| **${prTitle}**