Support reading commits inside the Dangerfile

.vscode/spellchecker.json

@@ -0,0 +1,19 @@
+{
+    "language": "en_US",
+    "ignoreWordsList": [
+        "GitHub",
+        "repo",
+        "Awesommmmee",
+        "linters",
+        "TODO",
+        "repl"
+    ],
+    "documentTypes": [
+        "markdown",
+        "latex",
+        "plaintext"
+    ],
+    "ignoreRegExp": [],
+    "ignoreFileExtensions": [],
+    "checkInterval": 5000
+}

README.md

@@ -80,7 +80,7 @@ git checkout branch-for-pr-1234
 DANGER_TEST_PR='1234' npm run danger
 ```
 
-assuming that your local filesystem matches up to that branch on github, this will be a good approximation of how danger will work when you integrate it into your ci system.
+assuming that your local file-system matches up to that branch on github, this will be a good approximation of how danger will work when you integrate it into your ci system.
 
 #### This thing is broken, I should help improve it!
 

changelog.md

@@ -1,6 +1,18 @@
 ### master
 
 //  Add your own contribution below
+
+* Adds support for `git.commits` and `github.commits` - orta
+
+  Why two? Well github.commits contains a bunch of github specific metadata ( e.g. GitHub user creds, 
+  commit comment counts. ) Chances are, you're always going to use `git.commits` however if you 
+  want more rich data, the GitHub one is available too. Here's an example:
+
+```js
+const merges = git.commits.filter(commit => commit.message.include("Merge Master"))
+if (merges.length) { fail("Please rebase your PR")}
+```
+
 * Allow debug dump output via `DEBUG=danger:*` environment variable - kwonoj
 * Adds surf-build ci provider - kwonoj
 * Forward environment variable to external module constructor - kwonoj
@@ -69,14 +81,14 @@
 * Improved error messaging around not including a `DANGER_GITHUB_API_TOKEN` in the ENV - nsfmc / orta
 * Adds support for getting the diff for a specific file from git: e.g.
 
-  ```js
+```js
 // Politely ask for their name on the entry too
 const changelogDiff = danger.git.diffForFile("changelog.md")
 const contributorName = danger.github.pr.user.login
 if (changelogDiff && changelogDiff.indexOf(contributorName) === -1) {
   warn("Please add your GitHub name to the changelog entry, so we can attribute you.")
 }
-  ```
+```
 
 ### 0.6.3
 

package.json

@@ -35,7 +35,7 @@
     ],
     "coveragePathIgnorePatterns": [
       "/node_modules/",
-      "(.test)\\.(ts|tsx)$",
+      "(.test)\\.(ts|tsx|js)$",
       "/distribution/.*\\.(ts|js)$"
     ]
   },

source/api/fetch.ts

@@ -50,7 +50,7 @@ export function api(url: string | any, init: any): Promise<any> {
     // Handle failing errors
     if (!response.ok) {
       process.exitCode = 1
-      console.error(`Request failed: ${response}`)
+      console.error(`Request failed: ${JSON.stringify(response, null, 2)}`)
       const msg = response.status === 0 ? "Network Error" : response.statusText
       throw new (Error as any)(response.status, msg, {response: response})
     }

source/dsl/Commit.ts

@@ -0,0 +1,25 @@
+/** A platform agnostic refernce to a Git commit */
+export interface GitCommit {
+  /** The SHA for the commit */
+  sha?: string,
+  /** Who wrote the commit */
+  author: GitCommitAuthor,
+  /** Who deployed the commit */
+  committer: GitCommitAuthor,
+  /** The commit message */
+  message: string,
+  /** Potential parent commits, and other assorted metadata */
+  tree: any,
+  /** SHAs for the commit's parents */
+  parents?: string[],
+}
+
+/** An author of a commit */
+export interface GitCommitAuthor {
+  /** The display name for the author */
+  name: string,
+  /** The authors email */
+  email: string,
+  /** ISO6801 date string */
+  date: string
+}

source/dsl/DangerDSL.ts

@@ -1,5 +1,5 @@
 import { GitDSL } from "../dsl/GitDSL"
-import { GitHubPRDSL, GitHubDSL } from "../dsl/GitHubDSL"
+import { GitHubDSL } from "../dsl/GitHubDSL"
 
 /**
  *  The Danger DSL provides the metadata for introspection
@@ -23,9 +23,8 @@ export interface DangerDSLType {
 export class DangerDSL {
   public readonly github: Readonly<GitHubDSL>
 
-  constructor(pr: GitHubPRDSL, public readonly git: GitDSL) {
-    this.github = {
-      pr
-    }
+  constructor(platformDSL: any, public readonly git: GitDSL) {
+    // As GitLab etc support is added this will need to be changed
+    this.github = platformDSL
   }
 }

source/dsl/GitDSL.ts

@@ -1,3 +1,5 @@
+import { GitCommit } from "./Commit"
+
 export interface GitDSL {
   /**
    * Filepaths with changes relative to the git root
@@ -18,5 +20,8 @@ export interface GitDSL {
   readonly deleted_files: Readonly<Array<string>>
 
   /** Offers the diff for a specific file */
-  diffForFile(filename: string): string | null
+  diffForFile(filename: string): string | null,
+
+  /** The Git commit metadata */
+  readonly commits: Readonly<Array<GitCommit>>
 }

source/dsl/GitHubDSL.ts

@@ -1,9 +1,27 @@
+import { GitCommit } from "./Commit"
+
+/** A GitHub specific implmentation of a git commit */
+export interface GitHubCommit {
+  /** The raw commit metadata */
+  commit: GitCommit,
+  /** The SHA for the commit */
+  sha: string,
+  /** the url for the commit on GitHub */
+  url: string,
+  /** The GitHub user who wrote the code */
+  author: GitHubUser,
+  /** The GitHub user who shipped the code */
+  committer: GitHubUser,
+  /** An array of parent commit shas */
+  parents: Array<any>
+}
+
+/** The GitHub metadata for your PR */
 export interface GitHubDSL {
-  /**
-   * The PR metadata for a code review session
-   * @type {GitHubPRDSL}
-   */
-  pr: GitHubPRDSL
+  /** The PR metadata for a code review session */
+  pr: GitHubPRDSL,
+  /** The github commit metadata for a code review session */
+  commits: Array<GitHubCommit>
 }
 
 /**
@@ -204,10 +222,10 @@ export interface GitHubPRDSL {
    */
   merged: boolean
 
-   /**
-   * The nuber of comments on the PR
-   * @type {number}
-   */
+  /**
+  * The nuber of comments on the PR
+  * @type {number}
+  */
   comments: number
 
   /**

source/platforms/FakePlatform.ts

@@ -10,16 +10,17 @@ export class FakePlatform implements Platform {
     this.name = "Fake"
   }
 
-  async getReviewInfo(): Promise<any> {
-    //noop
+  async getPlatformDSLRepresentation(): Promise<any> {
+    return {}
   }
 
   async getReviewDiff(): Promise<GitDSL> {
     return {
       modified_files: [],
       created_files: [],
       deleted_files: [],
-      diffForFile: () => ""
+      diffForFile: () => "",
+      commits: []
     }
   }
 

source/platforms/GitHub.ts

@@ -1,5 +1,8 @@
 import { GitDSL } from "../dsl/GitDSL"
 import { CISource } from "../ci_source/ci_source"
+import { GitCommit } from "../dsl/Commit"
+import { GitHubCommit, GitHubDSL } from "../dsl/GitHubDSL"
+
 import * as parseDiff from "parse-diff"
 import * as includes from "lodash.includes"
 import * as find from "lodash.find"
@@ -12,7 +15,8 @@ import * as os from "os"
 
 export type APIToken = string
 
-// TODO: Cache PR JSON
+// TODO: Cache PR JSON?
+// TODO: Separate out a GitHub API client?
 
 /** This represent the GitHub API, and conforming to the Platform Interface */
 
@@ -45,14 +49,16 @@ export class GitHub {
    */
   async getReviewDiff(): Promise<GitDSL> {
     const diffReq = await this.getPullRequestDiff()
+    const getCommitsResponse = await this.getPullRequestCommits()
+    const getCommits = await getCommitsResponse.json()
+
     const diff = await diffReq.text()
 
     const fileDiffs: Array<any> = parseDiff(diff)
 
     const addedDiffs = fileDiffs.filter((diff: any) => diff["new"])
     const removedDiffs = fileDiffs.filter((diff: any) => diff["deleted"])
     const modifiedDiffs = fileDiffs.filter((diff: any) => !includes(addedDiffs, diff) && !includes(removedDiffs, diff))
-
     return {
       modified_files: modifiedDiffs.map((d: any) => d.to),
       created_files: addedDiffs.map((d: any) => d.to),
@@ -65,7 +71,39 @@ export class GitHub {
           .reduce((a: any, b: any) => a.concat(b), [])
         const lines = changes.map((c: any) => c.content)
         return lines.join(os.EOL)
-      }
+      },
+      commits: getCommits.map(this.githubCommitToGitCommit)
+    }
+  }
+
+  /**
+   * Returns the `github` object on the Danger DSL
+   *
+   * @returns {Promise<GitHubDSL>} JSON response of the DSL
+   */
+  async getPlatformDSLRepresentation(): Promise<GitHubDSL> {
+    const pr = await this.getReviewInfo()
+    const commits = await this.getPullRequestCommits()
+    return {
+      pr,
+      commits
+    }
+  }
+
+  /**
+   * Returns the response for the new comment
+   *
+   * @param {GitHubCommit} ghCommit A GitHub based commit
+   * @returns {GitCommit} a Git commit representation without GH metadata
+   */
+  githubCommitToGitCommit(ghCommit: GitHubCommit): GitCommit {
+    return {
+      sha: ghCommit.sha,
+      parents: ghCommit.parents.map(p => p.sha),
+      author: ghCommit.commit.author,
+      committer: ghCommit.commit.committer,
+      message: ghCommit.commit.message,
+      tree: ghCommit.commit.tree
     }
   }
 
@@ -185,6 +223,12 @@ export class GitHub {
     return this.get(`repos/${repo}/pulls/${prID}`)
   }
 
+   getPullRequestCommits(): Promise<any> {
+    const repo = this.ciSource.repoSlug
+    const prID = this.ciSource.pullRequestID
+    return this.get(`repos/${repo}/pulls/${prID}/commits`)
+  }
+
   async getUserInfo(): Promise<any> {
     const response: any = await this.get("user")
     return await response.json()

source/platforms/_tests/GitHub.test.ts

@@ -1,4 +1,5 @@
 import { GitHub } from "../GitHub"
+import { GitCommit } from "../../dsl/Commit"
 import { FakeCI } from "../../ci_source/providers/Fake"
 import { readFileSync } from "fs"
 import { resolve } from "path"
@@ -60,20 +61,22 @@ describe("with fixtured data", () => {
     const mockSource = new FakeCI({})
     const github = new GitHub("Token", mockSource)
     github.getPullRequestInfo = await requestWithFixturedJSON("github_pr.json")
+    github.getPullRequestCommits = await requestWithFixturedJSON("github_commits.json")
 
     const info = await github.getReviewInfo()
     expect(info.title).toEqual("Adds support for showing the metadata and trending Artists to a Gene VC")
   })
 
   describe("the dangerfile gitDSL", async () => {
-    let github = {}
+    let github: GitHub = {} as any
     beforeEach(async () => {
-      github = new GitHub("Token", new FakeCI({}));
-      (github as any).getPullRequestDiff = await requestWithFixturedContent("github_diff.diff")
+      github = new GitHub("Token", new FakeCI({}))
+      github.getPullRequestDiff = await requestWithFixturedContent("github_diff.diff")
+      github.getPullRequestCommits = await requestWithFixturedJSON("github_commits.json")
     })
 
     it("sets the modified/created/deleted", async () => {
-    const gitDSL = await (github as any).getReviewDiff()
+      const gitDSL = await github.getReviewDiff()
 
       expect(gitDSL.modified_files).toEqual(["CHANGELOG.md", "data/schema.graphql", "data/schema.json", "externals/metaphysics", "lib/__mocks__/react-relay.js", "lib/components/artist/about.js", "lib/components/gene/header.js", "lib/containers/__tests__/__snapshots__/gene-tests.js.snap", "lib/containers/__tests__/gene-tests.js", "lib/containers/gene.js", "tsconfig.json"]) //tslint:disable-line:max-line-length
 
@@ -84,9 +87,33 @@ describe("with fixtured data", () => {
 
     it("shows the diff for a specific file", async () => {
       const expected = ` - [dev] Updates Flow to 0.32 - orta${EOL} - [dev] Updates React to 0.34 - orta${EOL} - [dev] Turns on "keychain sharing" to fix a keychain bug in sim - orta${EOL}+- GeneVC now shows about information, and trending artists - orta${EOL} ${EOL} ### 1.1.0-beta.2${EOL} ` //tslint:disable-line:max-line-length
-      const gitDSL = await (github as any).getReviewDiff()
+      const gitDSL = await github.getReviewDiff()
 
       expect(gitDSL.diffForFile("CHANGELOG.md")).toEqual(expected)
     })
+
+    it("sets up commit data correctly", async () => {
+      const exampleCommit: GitCommit = {
+        "author":  {
+          "date": "2016-09-30T13:52:14Z",
+          "email": "orta.therox@gmail.com",
+          "name": "Orta Therox",
+        },
+        "committer":  {
+          "date": "2016-09-30T13:52:14Z",
+          "email": "orta.therox@gmail.com",
+          "name": "Orta Therox",
+        },
+        "message": "WIP on Gene",
+        "parents":  ["98f3e73f5e419f3af9ab928c86312f28a3c87475"],
+        "sha": "13da2c844def1f4262ee440bd86fb2a3b021718b",
+        "tree": {
+          "sha": "d1b7448d7409093054efbb06ae12d1ffb002b956",
+          "url": "https://api.github.com/repos/artsy/emission/git/trees/d1b7448d7409093054efbb06ae12d1ffb002b956",
+        },
+      }
+      const gitDSL = await github.getReviewDiff()
+      expect(gitDSL.commits[0]).toEqual(exampleCommit)
+    })
   })
 })

source/platforms/platform.ts

@@ -32,8 +32,8 @@ export interface Platform {
   readonly name: string
   /** Used internally for getting PR/Repo metadata */
   readonly ciSource: CISource
-  /** Pulls in the Code Review Metadata for inspection */
-  getReviewInfo: () => Promise<any>
+  /** Pulls in the platform specific metadata for inspection */
+  getPlatformDSLRepresentation: () => Promise<any>
   /** Pulls in the Code Review Diff, and offers a succinct user-API for it */
   getReviewDiff: () => Promise<GitDSL>
   /** Creates a comment on the PR */

source/runner/DangerfileRunner.ts

@@ -82,7 +82,10 @@ export async function runDangerfileEnvironment(filename: Path, environment: Dang
  */
 export function updateDangerfile(filename: Path) {
   const contents = fs.readFileSync(filename).toString()
-  fs.writeFileSync(filename, cleanDangerfile(contents))
+  const cleanedDangerFile = cleanDangerfile(contents)
+  if (contents !== cleanedDangerFile) {
+    fs.writeFileSync(filename, cleanDangerfile(contents))
+  }
 }
 
 // https://regex101.com/r/dUq4yB/1