Some code cleanup

source/commands/utils/runDangerSubprocess.ts

@@ -68,7 +68,7 @@ const runDangerSubprocess = (subprocessName: string[], dslJSON: DangerDSLJSONTyp
       }
     }
     const danger = await jsonToDSL(dslJSON)
-    await exec.handleResults(results, danger)
+    await exec.handleResults(results, danger.git)
   })
 }
 

source/danger.d.ts

@@ -502,34 +502,34 @@ declare module "danger" {
   }
 
   export function inlineResults(results: DangerResults): DangerResults {
-    return sortResults({
+    return {
       fails: results.fails.filter(m => isInline(m)),
       warnings: results.warnings.filter(m => isInline(m)),
       messages: results.messages.filter(m => isInline(m)),
       markdowns: results.markdowns.filter(m => isInline(m)),
-    })
+    }
   }
 
   export function regularResults(results: DangerResults): DangerResults {
-    return sortResults({
+    return {
       fails: results.fails.filter(m => !isInline(m)),
       warnings: results.warnings.filter(m => !isInline(m)),
       messages: results.messages.filter(m => !isInline(m)),
       markdowns: results.markdowns.filter(m => !isInline(m)),
-    })
+    }
   }
 
   export function mergeResults(results1: DangerResults, results2: DangerResults): DangerResults {
-    return sortResults({
+    return {
       fails: results1.fails.concat(results2.fails),
       warnings: results1.warnings.concat(results2.warnings),
       messages: results1.messages.concat(results2.messages),
       markdowns: results1.markdowns.concat(results2.markdowns),
-    })
+    }
   }
 
   export function sortResults(results: DangerResults): DangerResults {
-    let sortByFile = (a: Violation, b: Violation): number => {
+    const sortByFile = (a: Violation, b: Violation): number => {
       if (a.file === undefined) {
         return -1
       }
@@ -570,16 +570,16 @@ declare module "danger" {
   }
 
   export function resultsIntoInlineResults(results: DangerResults): DangerInlineResults[] {
-    let dangerInlineResults: DangerInlineResults[] = []
+    const dangerInlineResults: DangerInlineResults[] = []
 
-    let violationsIntoInlineResults = (kind: string) => {
+    const violationsIntoInlineResults = (kind: string) => {
       for (let violation of results[kind]) {
         if (violation.file && violation.line) {
-          let findInlineResult = dangerInlineResults.find(r => r.file == violation.file && r.line == violation.line)
+          const findInlineResult = dangerInlineResults.find(r => r.file == violation.file && r.line == violation.line)
           if (findInlineResult) {
             findInlineResult[kind].push(violation.message)
           } else {
-            let inlineResult = {
+            const inlineResult = {
               file: violation.file,
               line: violation.line,
               fails: [],
@@ -599,7 +599,7 @@ declare module "danger" {
   }
 
   export function inlineResultsIntoResults(inlineResults: DangerInlineResults): DangerResults {
-    let messageToViolation = (message: string): Violation => {
+    const messageToViolation = (message: string): Violation => {
       return { message: message, file: inlineResults.file, line: inlineResults.line }
     }
 

source/dsl/DangerResults.ts

@@ -80,34 +80,34 @@ export const emptyDangerResults = {
 }
 
 export function inlineResults(results: DangerResults): DangerResults {
-  return sortResults({
+  return {
     fails: results.fails.filter(m => isInline(m)),
     warnings: results.warnings.filter(m => isInline(m)),
     messages: results.messages.filter(m => isInline(m)),
     markdowns: results.markdowns.filter(m => isInline(m)),
-  })
+  }
 }
 
 export function regularResults(results: DangerResults): DangerResults {
-  return sortResults({
+  return {
     fails: results.fails.filter(m => !isInline(m)),
     warnings: results.warnings.filter(m => !isInline(m)),
     messages: results.messages.filter(m => !isInline(m)),
     markdowns: results.markdowns.filter(m => !isInline(m)),
-  })
+  }
 }
 
 export function mergeResults(results1: DangerResults, results2: DangerResults): DangerResults {
-  return sortResults({
+  return {
     fails: results1.fails.concat(results2.fails),
     warnings: results1.warnings.concat(results2.warnings),
     messages: results1.messages.concat(results2.messages),
     markdowns: results1.markdowns.concat(results2.markdowns),
-  })
+  }
 }
 
 export function sortResults(results: DangerResults): DangerResults {
-  let sortByFile = (a: Violation, b: Violation): number => {
+  const sortByFile = (a: Violation, b: Violation): number => {
     if (a.file === undefined) {
       return -1
     }
@@ -148,16 +148,16 @@ export function sortResults(results: DangerResults): DangerResults {
 }
 
 export function resultsIntoInlineResults(results: DangerResults): DangerInlineResults[] {
-  let dangerInlineResults: DangerInlineResults[] = []
+  const dangerInlineResults: DangerInlineResults[] = []
 
-  let violationsIntoInlineResults = (kind: string) => {
+  const violationsIntoInlineResults = (kind: string) => {
     for (let violation of results[kind]) {
       if (violation.file && violation.line) {
-        let findInlineResult = dangerInlineResults.find(r => r.file == violation.file && r.line == violation.line)
+        const findInlineResult = dangerInlineResults.find(r => r.file == violation.file && r.line == violation.line)
         if (findInlineResult) {
           findInlineResult[kind].push(violation.message)
         } else {
-          let inlineResult = {
+          const inlineResult = {
             file: violation.file,
             line: violation.line,
             fails: [],
@@ -177,7 +177,7 @@ export function resultsIntoInlineResults(results: DangerResults): DangerInlineRe
 }
 
 export function inlineResultsIntoResults(inlineResults: DangerInlineResults): DangerResults {
-  let messageToViolation = (message: string): Violation => {
+  const messageToViolation = (message: string): Violation => {
     return { message: message, file: inlineResults.file, line: inlineResults.line }
   }
 

source/dsl/_tests/DangerResults.test.ts

@@ -18,65 +18,65 @@ import {
 
 describe("DangerResults into DangerInlineResults", () => {
   it("transforms empty results into empty inlineResults", () => {
-    let results = resultsIntoInlineResults(emptyDangerResults)
+    const results = resultsIntoInlineResults(emptyDangerResults)
 
     expect(results).toMatchSnapshot()
   })
 
   it("transforms single-violation results into one inlineResults", () => {
-    let results = resultsIntoInlineResults(singleViolationSingleFileResults)
+    const results = resultsIntoInlineResults(singleViolationSingleFileResults)
 
     expect(results).toMatchSnapshot()
   })
 
   it("transforms multiple-violation results into one inlineResults", () => {
-    let results = resultsIntoInlineResults(multipleViolationSingleFileResults)
+    const results = resultsIntoInlineResults(multipleViolationSingleFileResults)
 
     expect(results).toMatchSnapshot()
   })
 
   it("transforms multiple-violation results into multiple inlineResults within one file", () => {
-    let results = resultsIntoInlineResults(multipleViolationsMultipleFilesResults)
+    const results = resultsIntoInlineResults(multipleViolationsMultipleFilesResults)
 
     expect(results).toMatchSnapshot()
   })
 })
 
 describe("DangerInlineResults into DangerResults", () => {
   it("transforms empty inlineResults into results", () => {
-    let results = inlineResultsIntoResults(emptyDangerInlineResults)
+    const results = inlineResultsIntoResults(emptyDangerInlineResults)
 
     expect(results).toMatchSnapshot()
   })
 
   it("transforms single-violation inlineResults into results", () => {
-    let results = inlineResultsIntoResults(singleViolationsInlineResults)
+    const results = inlineResultsIntoResults(singleViolationsInlineResults)
 
     expect(results).toMatchSnapshot()
   })
 
   it("transforms multiple-violation inlineResults into results", () => {
-    let results = inlineResultsIntoResults(multipleViolationsInlineResults)
+    const results = inlineResultsIntoResults(multipleViolationsInlineResults)
 
     expect(results).toMatchSnapshot()
   })
 })
 
 describe("DangerResults operations", () => {
   it("merges two results correcly", () => {
-    let results = mergeResults(singleViolationSingleFileResults, multipleViolationSingleFileResults)
+    const results = mergeResults(singleViolationSingleFileResults, multipleViolationSingleFileResults)
 
     expect(results).toMatchSnapshot()
   })
 
   it("filters results to get only inline violations", () => {
-    let results = inlineResults(regularAndInlineViolationsResults)
+    const results = inlineResults(regularAndInlineViolationsResults)
 
     expect(results).toMatchSnapshot()
   })
 
   it("filters results to get only regular violations", () => {
-    let results = regularResults(regularAndInlineViolationsResults)
+    const results = regularResults(regularAndInlineViolationsResults)
 
     expect(results).toMatchSnapshot()
   })

source/platforms/GitHub.ts

@@ -129,7 +129,6 @@ export class GitHub implements Platform {
     let commitId = git.commits[git.commits.length - 1].sha
 
     return this.findPositionForInlineComment(git, line, path).then(position => {
-      console.log("position: " + position + ", file: " + path + ", line: " + line)
       return this.api.postInlinePRComment(comment, commitId, path, position)
     })
   }
@@ -159,9 +158,6 @@ export class GitHub implements Platform {
     })
   }
 
-  // In Danger RB we support a danger_id property,
-  // this should be handled at some point
-
   /**
    * Deletes the main Danger comment, used when you have
    * fixed all your failures.

source/platforms/github/_tests/_github_api.test.ts

@@ -100,7 +100,7 @@ describe("API testing", () => {
 
   it("postInlinePRComment success", async () => {
     api.fetch = fetchJSON
-    let expectedJSON = {
+    const expectedJSON = {
       api: "https://api.github.com/repos/artsy/emission/pulls/1/comments",
       headers: {
         Authorization: "token ABCDE",
@@ -115,7 +115,7 @@ describe("API testing", () => {
 
   it("postInlinePRComment error", async () => {
     api.fetch = fetchErrorJSON
-    let expectedJSON = {
+    const expectedJSON = {
       api: "https://api.github.com/repos/artsy/emission/pulls/1/comments",
       headers: {
         Authorization: "token ABCDE",

source/platforms/github/_tests/_github_git.test.ts

@@ -146,12 +146,12 @@ describe("the dangerfile gitDSL", async () => {
   })
 
   it("finds the position of file/line for inline comment with one chunk", async () => {
-    let position = await github.findPositionForInlineComment(gitDSL, 9, "tsconfig.json")
+    const position = await github.findPositionForInlineComment(gitDSL, 9, "tsconfig.json")
     expect(position).toBe(6)
   })
 
   it("finds the position of file/line for inline comment with two chunks", async () => {
-    let position = await github.findPositionForInlineComment(gitDSL, 28, "lib/containers/gene.js")
+    const position = await github.findPositionForInlineComment(gitDSL, 28, "lib/containers/gene.js")
     expect(position).toBe(19)
   })
 

source/runner/Executor.ts

@@ -1,5 +1,4 @@
 import { contextForDanger, DangerContext } from "./Dangerfile"
-import { DangerDSL, DangerDSLType } from "../dsl/DangerDSL"
 import { CISource } from "../ci_source/ci_source"
 import { Platform } from "../platforms/platform"
 import {
@@ -11,6 +10,7 @@ import {
   resultsIntoInlineResults,
   emptyDangerResults,
   inlineResultsIntoResults,
+  sortResults,
 } from "../dsl/DangerResults"
 import { template as githubResultsTemplate } from "./templates/githubIssueTemplate"
 import { template as bitbucketServerTemplate } from "./templates/bitbucketServerTemplate"
@@ -23,6 +23,7 @@ import { DangerRunner } from "./runners/runner"
 import { jsonToDSL } from "./jsonToDSL"
 import { jsonDSLGenerator } from "./dslGenerator"
 import { GitDSL } from "../dsl/GitDSL"
+import { DangerDSL } from "../dsl/DangerDSL"
 
 // This is still badly named, maybe it really should just be runner?
 
@@ -87,7 +88,7 @@ export class Executor {
       results = this.resultsForError(error)
     }
 
-    await this.handleResults(results, runtime.danger)
+    await this.handleResults(results, runtime.danger.git)
     return results
   }
 
@@ -107,12 +108,12 @@ export class Executor {
    *
    * @param {DangerResults} results a JSON representation of the end-state for a Danger run
    */
-  async handleResults(results: DangerResults, danger: DangerDSLType) {
+  async handleResults(results: DangerResults, git: GitDSL) {
     this.d(`Got Results back, current settings`, this.options)
     if (this.options.stdoutOnly || this.options.jsonOnly) {
       await this.handleResultsPostingToSTDOUT(results)
     } else {
-      await this.handleResultsPostingToPlatform(results, danger)
+      await this.handleResultsPostingToPlatform(results, git)
     }
   }
   /**
@@ -173,7 +174,7 @@ export class Executor {
    * @param {DangerResults} results a JSON representation of the end-state for a Danger run
    */
   // TODO: Instead of danger, pass gitDSL
-  async handleResultsPostingToPlatform(results: DangerResults, danger: DangerDSLType) {
+  async handleResultsPostingToPlatform(results: DangerResults, git: GitDSL) {
     // Delete the message if there's nothing to say
     const { fails, warnings, messages, markdowns } = results
 
@@ -208,9 +209,9 @@ export class Executor {
         console.log("Found only messages, passing those to review.")
       }
       const inline = inlineResults(results)
-      const inlineLeftovers = await this.sendInlineComments(inline, danger.git)
+      const inlineLeftovers = await this.sendInlineComments(inline, git)
       const regular = regularResults(results)
-      const mergedResults = mergeResults(regular, inlineLeftovers)
+      const mergedResults = sortResults(mergeResults(regular, inlineLeftovers))
       const comment = process.env["DANGER_BITBUCKETSERVER_HOST"]
         ? bitbucketServerTemplate(dangerID, mergedResults)
         : githubResultsTemplate(dangerID, mergedResults)
@@ -233,17 +234,11 @@ export class Executor {
   sendInlineComments(results: DangerResults, git: GitDSL): Promise<DangerResults> {
     // TODO: Get current inline comments, if any of the old ones is not present
     // in the new ones - delete.
-    let inlineResults = resultsIntoInlineResults(results)
-    let promises = inlineResults.map(inlineResult => {
+    const inlineResults = resultsIntoInlineResults(results)
+    const promises = inlineResults.map(inlineResult => {
       return this.sendInlineComment(git, inlineResult)
-        .then(response => {
-          console.log("response from inline comment: " + JSON.stringify(response, null, 4))
-          return emptyDangerResults
-        })
-        .catch(error => {
-          console.log("error from inline comment: " + JSON.stringify(error, null, 4))
-          return inlineResultsIntoResults(inlineResult)
-        })
+        .then(_r => emptyDangerResults)
+        .catch(_e => inlineResultsIntoResults(inlineResult))
     })
     return Promise.all(promises).then(dangerResults => {
       return new Promise<DangerResults>(resolve => {
@@ -253,7 +248,7 @@ export class Executor {
   }
 
   async sendInlineComment(git: GitDSL, inlineResults: DangerInlineResults): Promise<any> {
-    let template = githubResultsTemplate(this.options.dangerID, inlineResultsIntoResults(inlineResults))
+    const template = githubResultsTemplate(this.options.dangerID, inlineResultsIntoResults(inlineResults))
     return await this.platform.createInlineComment(git, template, inlineResults.file, inlineResults.line)
   }