Refactor GitHub api (#82)

This refactors the GitHub API and related functions in preparation for GitLab support

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);

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();
+  const newPrNo = await configureRepository();
-  logger.info(`Created PR #${newPr.number} for configuration`);
+  logger.info(`Created PR #${newPrNo} for configuration`);
   return false;
 }
 

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();
+    const pr = await api.getBranchPr(branchName);
-    if (pr.mergeable_state === 'dirty') {
+    if (pr && pr.isUnmergeable) {
       logger.verbose(`Existing PR #${pr.number} is not mergeable`);
-      if (pr.additions * pr.deletions === 1) {
+      if (pr.canRebase) {
-        // No other changes except ours
+        // Only supported by GitHub
-        logger.info(`Rebasing ${branchName}`);
         // 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);