From ee27fd5e1fbe3b75d2a5e81162cf9f1a18ef5f10 Mon Sep 17 00:00:00 2001 From: Rhys Arkins Date: Wed, 1 Feb 2017 13:50:28 +0100 Subject: [PATCH] Refactor GitHub api (#82) This refactors the GitHub API and related functions in preparation for GitLab support --- lib/api/github.js | 36 ++++++++++++++++++++++++++++++------ lib/index.js | 10 +++++----- lib/worker.js | 35 +++++++++-------------------------- 3 files changed, 44 insertions(+), 37 deletions(-) diff --git a/lib/api/github.js b/lib/api/github.js index 9eebddef36..f644148b91 100644 --- a/lib/api/github.js +++ b/lib/api/github.js @@ -116,13 +116,15 @@ async function updateBranch(branchName, commit) { }); } -// Returns the Pull Request number for a branch. Null if not exists. +// Returns the Pull Request for a branch. Null if not exists. async function getBranchPr(branchName) { + logger.debug(`getBranchPr(${branchName})`); const gotString = `repos/${config.repoName}/pulls?` + `state=open&base=${config.defaultBranch}&head=${config.owner}:${branchName}`; const res = await ghGot(gotString); if (res.body.length) { - return res.body[0]; + const prNo = res.body[0].number; + return getPr(prNo); } return null; } @@ -164,8 +166,11 @@ async function findPr(branchName, prTitle, state = 'all') { const res = await ghGot(urlString); let pr = null; res.body.forEach((result) => { - if (result.title === prTitle) { + if (!prTitle || result.title === prTitle) { pr = result; + if (pr.state === 'closed') { + pr.isClosed = true; + } } }); return pr; @@ -180,14 +185,33 @@ async function checkForClosedPr(branchName, prTitle) { return res.body.some(pr => pr.title === prTitle && pr.head.label === `${config.owner}:${branchName}`); } +// Creates PR and returns PR number async function createPr(branchName, title, body) { return (await ghGot.post(`repos/${config.repoName}/pulls`, { body: { title, head: branchName, base: config.defaultBranch, body }, - })).body; + })).body.number; } +// Gets details for a PR async function getPr(prNo) { - return (await ghGot(`repos/${config.repoName}/pulls/${prNo}`)).body; + if (!prNo) { + return null; + } + const pr = (await ghGot(`repos/${config.repoName}/pulls/${prNo}`)).body; + if (!pr) { + return null; + } + // Harmonise PR values + if (pr.state === 'closed') { + pr.isClosed = true; + } + if (pr.mergeable_state === 'dirty') { + pr.isUnmergeable = true; + } + if (pr.additions * pr.deletions === 1) { + pr.canRebase = true; + } + return pr; } async function updatePr(prNo, title, body) { @@ -244,7 +268,7 @@ async function writeFile(branchName, oldFileSHA, filePath, fileContents, message // Add a new commit, return SHA async function commitFile(fileName, fileContents, message, parentBranch = config.defaultBranch) { - logger.debug(`commitFile(${fileName}, fileContents, message, ${parentBranch})`); + logger.debug(`commitFile('${fileName}', fileContents, message, '${parentBranch})'`); const parentCommit = await getBranchCommit(parentBranch); const parentTree = await getCommitTree(parentCommit); const blob = await createBlob(fileContents); diff --git a/lib/index.js b/lib/index.js index 24970d9ff2..f5c8177f17 100644 --- a/lib/index.js +++ b/lib/index.js @@ -1,7 +1,7 @@ const stringify = require('json-stringify-pretty-compact'); const logger = require('./logger'); const configParser = require('./config'); -const github = require('./api/github'); +const githubApi = require('./api/github'); const defaultsParser = require('./config/defaults'); // Require main source @@ -34,7 +34,7 @@ async function processRepo(repo) { // Take a copy of the config, as we will modify it const config = Object.assign({}, repo); if (config.platform === 'github') { - api = github; + api = githubApi; } else { // Gitlab will be added here logger.error(`Unknown platform ${config.platform} for repository ${repo.repository}`); return; @@ -85,7 +85,7 @@ async function checkIfConfigured(config) { const pr = await api.findPr('renovate/configure', 'Configure Renovate'); if (pr) { logger.debug('Found \'Configure Renovate\' PR'); - if (pr.state === 'closed') { + if (pr.isClosed) { logger.debug('Closed Configure Renovate PR found - continuing'); return true; } @@ -93,8 +93,8 @@ async function checkIfConfigured(config) { logger.info(`Close PR #${pr.number} first`); return false; } - const newPr = await configureRepository(); - logger.info(`Created PR #${newPr.number} for configuration`); + const newPrNo = await configureRepository(); + logger.info(`Created PR #${newPrNo} for configuration`); return false; } diff --git a/lib/worker.js b/lib/worker.js index 5cd6b734fe..7bf24ff702 100644 --- a/lib/worker.js +++ b/lib/worker.js @@ -1,7 +1,7 @@ const logger = require('winston'); const changelog = require('changelog'); const stringify = require('json-stringify-pretty-compact'); -const github = require('./api/github'); +const githubApi = require('./api/github'); const handlebars = require('./helpers/handlebars'); const versionsHelper = require('./helpers/versions'); const packageJson = require('./helpers/package-json'); @@ -15,7 +15,7 @@ module.exports = renovate; // This function manages the queue per-package file async function renovate(repoName, packageFile, packageConfig) { if (packageConfig.platform === 'github') { - api = github; + api = githubApi; } // Other platforms like Gitlab will go here // Initialize globals config = Object.assign({}, packageConfig); @@ -133,16 +133,6 @@ async function updateDependency(upgrade) { return false; } - async function getBranchPr() { - const branchPr = await api.getBranchPr(branchName); - if (branchPr) { - logger.debug(`Found open PR for ${branchName}`); - return api.getPr(branchPr.number); - } - logger.debug(`Didn't find open PR for ${branchName}`); - return null; - } - async function ensureBranch() { const branchExists = await api.branchExists(branchName); if (branchExists) { @@ -164,16 +154,16 @@ async function updateDependency(upgrade) { // By default, we'll add a commit to the existing branch if necessary let parentBranch = branchName; logger.debug(`Checking if branch ${branchName} needs updating`); - const pr = await getBranchPr(); - if (pr.mergeable_state === 'dirty') { + const pr = await api.getBranchPr(branchName); + if (pr && pr.isUnmergeable) { logger.verbose(`Existing PR #${pr.number} is not mergeable`); - if (pr.additions * pr.deletions === 1) { - // No other changes except ours - logger.info(`Rebasing ${branchName}`); + if (pr.canRebase) { + // Only supported by GitHub // Setting parentBranch to undefined means that we'll use the default branch parentBranch = undefined; + logger.debug(`Rebasing branch ${branchName}`); } else { - logger.warning(`Cannot rebase ${branchName} as it has been modified`); + logger.debug(`Cannot rebase branch ${branchName}`); } } const existingContent = await api.getFileContent(config.packageFile, parentBranch); @@ -235,13 +225,6 @@ async function updateDependency(upgrade) { logger.error(`${depName} failed to ensure PR: ${error}`); } - // Create PR based on current state - async function createPr() { - const newPr = await api.createPr(branchName, prTitle, prBody); - logger.info(`${depName}: Created PR #${newPr.number}`); - return newPr.number; - } - // Update PR based on current state async function updatePr(existingPr) { logger.debug(`updatePr: ${existingPr.number}`); @@ -254,7 +237,7 @@ async function updateDependency(upgrade) { if (!existingPr) { logger.debug(`Didn't find existing PR for branch ${branchName}`); // We need to create a new PR - const prNo = await createPr(); + const prNo = await api.createPr(branchName, prTitle, prBody); await addLabels(prNo); await addAssignees(prNo); await addReviewers(prNo);