Merge pull request #490 from danger/platform_content

Expose a get file contents on the platform interface

CHANGELOG.md

@@ -10,14 +10,34 @@
 
 ## Master
 
-* Adds a new command `danger local`
+* Adds a new command `danger local`.
 
   This command will look between the current branch and master
   and use that to evaluate a dangerfile. This is aimed specifically at
   tools like git commit hooks, and for people who don't do code review.
 
   `danger.github` will be falsy in this context, so you could share a dangerfile
-  between your CI + code Review. - [@orta][]
+  between `danger local` and `danger ci`.
+
+  When I thought about how to use it on Danger JS, I opted to make another Dangerfile and import it at the end of
+  the main Dangerfile. This new Dangerfile only contains rules which can run with just `danger.git`, e.g. CHANGELOG/README
+  checks. I called it `dangerfile.lite.ts`.
+
+  Our setup looks like:
+
+  ```json
+  "scripts": {
+    "prepush": "yarn build; yarn danger:prepush",
+    "danger:prepush": "yarn danger local --dangerfile dangerfile.lite.ts"
+    // [...]
+  ```
+
+You'll need to have [husky](https://www.npmjs.com/package/husky) installed for this to work. - [@orta][]
+
+* STDOUT formatting has been improved, which is the terminal only version of
+  Danger's typical GitHub comment style system. It's used in `danger pr`, `danger ci --stdout`
+  and `danger local`. - [@orta][]
+* Exposed a get file contents for the platform abstraction so that Peril can work on many platforms in the future - [@orta][]
 
 ### 3.0.5
 

dangerfile.lite.ts

@@ -5,8 +5,12 @@ declare var danger: DangerDSLType
 declare function warn(params: string): void
 
 const hasChangelog = danger.git.modified_files.includes("CHANGELOG.md")
-if (!hasChangelog) {
-  warn("Please add a changelog entry for your changes.")
+const isTrivial = danger.github && (danger.github.pr.body + danger.github.pr.title).includes("#trivial")
+
+if (!hasChangelog && !isTrivial) {
+  warn(
+    "Please add a changelog entry for your changes. You can find it in `CHANGELOG.md` \n\n Please add your change and name to the master section."
+  )
 }
 
 import dtsGenerator from "./scripts/danger-dts"

dangerfile.ts

@@ -13,18 +13,14 @@ declare function fail(params: string): void
 // declare function schedule(promise: () => Promise<any | void>): void
 // declare function schedule(callback: (resolve: any) => void): void
 
-import * as fs from "fs"
-
 const checkREADME = async () => {
   // Request a CHANGELOG entry if not declared #trivial
   const hasChangelog = danger.git.modified_files.includes("CHANGELOG.md")
   const isTrivial = (danger.github.pr.body + danger.github.pr.title).includes("#trivial")
   const isGreenkeeper = danger.github.pr.user.login === "greenkeeper"
 
+  // Politely ask for their name on the entry too
   if (!hasChangelog && !isTrivial && !isGreenkeeper) {
-    warn("Please add a changelog entry for your changes, by adding a note in the master section to `CHANGELOG.md`.")
-
-    // Politely ask for their name on the entry too
     const changelogDiff = await danger.git.diffForFile("CHANGELOG.md")
     const contributorName = danger.github.pr.user.login
     if (changelogDiff && changelogDiff.diff.includes(contributorName)) {
@@ -40,37 +36,5 @@ yarn()
 import jest from "danger-plugin-jest"
 jest()
 
-// Some good old-fashioned maintainance upkeep
-
-// Ensure the danger.d.ts is always up to date inside this repo.
-// This also serves as the "one true DSL" for a Danger run against a PR
-// which tools can then work against.
-
-// debugger
-
-import dtsGenerator from "./scripts/danger-dts"
-const currentDTS = dtsGenerator()
-const savedDTS = fs.readFileSync("source/danger.d.ts").toString()
-if (currentDTS !== savedDTS) {
-  const message = "There are changes to the Danger DSL which are not reflected in the current danger.d.ts."
-  const idea = "Please run <code>yarn declarations</code> and update this PR."
-  fail(`${message}<br/><i>${idea}</i>`)
-}
-
-// Always ensure we name all CI providers in the README. These
-// regularly get forgotten on a PR adding a new one.
-const sentence = danger.utils.sentence
-
-import { realProviders } from "./source/ci_source/providers"
-const readme = fs.readFileSync("README.md").toString()
-const names = realProviders.map(p => new p({}).name)
-const missing = names.filter(n => !readme.includes(n))
-if (missing.length) {
-  warn(`These providers are missing from the README: ${sentence(missing)}`)
-}
-
-danger.github.utils.fileContents("scripts/run-fixtures.js").then(fixtures => {
-  if (fixtures.includes("const writeResults = true")) {
-    fail("Fixtures test script is still in write mode, edit `scripts/run-fixtures.js`.")
-  }
-})
+// Re-run the git push hooks
+import "./dangerfile.lite"

package.json

@@ -54,7 +54,7 @@
     "docs":
       "yarn run docs:cp_defs; yarn typedoc --ignoreCompilerErrors --mode modules --json docs/js_ref_dsl_docs.json --includeDeclarations source",
     "dts-lint": "yarn run declarations && yarn dtslint types",
-    "danger:prepush": "node  distribution/commands/danger.js local --dangerfile dangerfile.lite.ts"
+    "danger:prepush": "node distribution/commands/danger.js local --dangerfile dangerfile.lite.ts"
   },
   "repository": {
     "type": "git",

source/platforms/FakePlatform.ts

@@ -1,6 +1,7 @@
 import { GitDSL } from "../dsl/GitDSL"
 import { CISource } from "../ci_source/ci_source"
 import { Platform } from "./platform"
+import { readFileSync } from "fs-extra"
 
 export class FakePlatform implements Platform {
   public readonly name: string
@@ -45,4 +46,6 @@ export class FakePlatform implements Platform {
   async updateStatus(_success: boolean, _message: string): Promise<boolean> {
     return true
   }
+
+  getFileContents = (path: string) => new Promise<string>(res => res(readFileSync(path, "utf8")))
 }

source/platforms/GitHub.ts

@@ -20,24 +20,20 @@ export class GitHub {
    *
    * @returns {Promise<any>} JSON representation
    */
-  async getReviewInfo(): Promise<GitHubPRDSL> {
-    return await this.api.getPullRequestInfo()
-  }
+  getReviewInfo = (): Promise<GitHubPRDSL> => this.api.getPullRequestInfo()
 
   /**
    * Get the Code Review diff representation
    *
    * @returns {Promise<GitDSL>} the git DSL
    */
-  async getPlatformGitRepresentation(): Promise<GitJSONDSL> {
-    return gitDSLForGitHub(this.api)
-  }
+  getPlatformGitRepresentation = (): Promise<GitJSONDSL> => gitDSLForGitHub(this.api)
 
   /**
    * Gets issue specific metadata for a PR
    */
 
-  async getIssue(): Promise<GitHubIssue> {
+  getIssue = async (): Promise<GitHubIssue> => {
     const issue = await this.api.getIssue()
     return issue || { labels: [] }
   }
@@ -47,7 +43,7 @@ export class GitHub {
    * then return true.
    */
 
-  async updateStatus(passed: boolean, message: string, url?: string): Promise<boolean> {
+  updateStatus = async (passed: boolean, message: string, url?: string): Promise<boolean> => {
     const ghAPI = this.api.getExternalAPI()
 
     const prJSON = await this.api.getPullRequestInfo()
@@ -73,7 +69,7 @@ export class GitHub {
    *
    * @returns {Promise<GitHubDSL>} JSON response of the DSL
    */
-  async getPlatformDSLRepresentation(): Promise<GitHubJSONDSL> {
+  getPlatformDSLRepresentation = async (): Promise<GitHubJSONDSL> => {
     const pr = await this.getReviewInfo()
     if (pr === {}) {
       process.exitCode = 1
@@ -110,9 +106,7 @@ export class GitHub {
    * @param {string} comment you want to post
    * @returns {Promise<any>} JSON response of new comment
    */
-  async createComment(comment: string): Promise<any> {
-    return this.api.postPRComment(comment)
-  }
+  createComment = (comment: string) => this.api.postPRComment(comment)
 
   // In Danger RB we support a danger_id property,
   // this should be handled at some point
@@ -123,7 +117,7 @@ export class GitHub {
    *
    * @returns {Promise<boolean>} did it work?
    */
-  async deleteMainComment(dangerID: string): Promise<boolean> {
+  deleteMainComment = async (dangerID: string): Promise<boolean> => {
     const commentIDs = await this.api.getDangerCommentIDs(dangerID)
     for (let commentID of commentIDs) {
       await this.api.deleteCommentWithID(commentID)
@@ -161,13 +155,15 @@ export class GitHub {
   /**
    * Converts the PR JSON into something easily used by the Github API client.
    */
-  APIMetadataForPR(pr: GitHubPRDSL): GitHubAPIPR {
+  APIMetadataForPR = (pr: GitHubPRDSL): GitHubAPIPR => {
     return {
       number: pr.number,
       repo: pr.base.repo.name,
       owner: pr.base.repo.owner.login,
     }
   }
+
+  getFileContents = this.api.fileContents
 }
 
 // This class should get un-classed, but for now we can expand by functions

source/platforms/LocalGit.ts

@@ -6,6 +6,7 @@ import { GitCommit } from "../dsl/Commit"
 import { localGetDiff } from "./git/localGetDiff"
 import { localGetFileAtSHA } from "./git/localGetFileAtSHA"
 import { localGetCommits } from "./git/localGetCommits"
+import { readFileSync } from "fs"
 
 export interface LocalGitOptions {
   base?: string
@@ -65,4 +66,6 @@ export class LocalGit implements Platform {
   async updateStatus(_success: boolean, _message: string): Promise<boolean> {
     return true
   }
+
+  getFileContents = (path: string) => new Promise<string>(res => res(readFileSync(path, "utf8")))
 }

source/platforms/github/GitHubAPI.ts

@@ -24,6 +24,8 @@ export class GitHubAPI {
   additionalHeaders: any
   private readonly d = debug("danger:GitHubAPI")
 
+  private pr: GitHubPRDSL
+
   constructor(public readonly repoMetadata: RepoMetaData, public readonly token?: APIToken) {
     // This allows Peril to DI in a new Fetch function
     // which can handle unique API edge-cases around integrations
@@ -130,11 +132,20 @@ export class GitHubAPI {
   }
 
   getPullRequestInfo = async (): Promise<GitHubPRDSL> => {
+    if (this.pr) {
+      return this.pr
+    }
     const repo = this.repoMetadata.repoSlug
     const prID = this.repoMetadata.pullRequestID
     const res = await this.get(`repos/${repo}/pulls/${prID}`)
+    const prDSL = (await res.json()) as GitHubPRDSL
+    this.pr = prDSL
 
-    return res.ok ? (res.json() as Promise<GitHubPRDSL>) : ({} as GitHubPRDSL)
+    if (res.ok) {
+      return prDSL
+    } else {
+      throw `Could not get PR Metadata for repos/${repo}/pulls/${prID}`
+    }
   }
 
   getPullRequestCommits = async (): Promise<any[]> => {

source/platforms/platform.ts

@@ -45,6 +45,8 @@ export interface Platform {
   updateOrCreateComment: (dangerID: string, newComment: string) => Promise<any>
   /** Sets the current PR's status */
   updateStatus: (passed: boolean, message: string, url?: string) => Promise<boolean>
+  /** Get the contents of a file at a path */
+  getFileContents: (path: string, slug?: string, ref?: string) => Promise<string>
 }
 
 /**

source/runner/Executor.ts

@@ -125,11 +125,11 @@ export class Executor {
       // Human-readable format
 
       const table = [
-        { name: "Failures", messages: fails.map(f => f.message) },
-        { name: "Warnings", messages: warnings.map(w => w.message) },
-        { name: "Messages", messages: messages.map(m => m.message) },
-        { name: "Markdowns", messages: markdowns },
-      ]
+        fails.length && { name: "Failures", messages: fails.map(f => f.message) },
+        warnings.length && { name: "Warnings", messages: warnings.map(w => w.message) },
+        messages.length && { name: "Messages", messages: messages.map(m => m.message) },
+        markdowns.length && { name: "Markdowns", messages: markdowns },
+      ].filter(r => r !== 0) as { name: string; messages: string[] }[]
 
       // Consider looking at getting the terminal width, and making it 60%
       // if over a particular size
@@ -142,14 +142,16 @@ export class Executor {
       if (fails.length > 0) {
         const s = fails.length === 1 ? "" : "s"
         const are = fails.length === 1 ? "is" : "are"
-        const message = chalk.underline("Failing the build")
-        console.log(`${message}, there ${are} ${fails.length} fail${s}.`)
+        const message = chalk.underline.red("Failing the build")
+        console.log(`Danger: ${message}, there ${are} ${fails.length} fail${s}.`)
         process.exitCode = 1
       } else if (warnings.length > 0) {
         const message = chalk.underline("not failing the build")
-        console.log(`Found only warnings, ${message}`)
+        console.log(`Danger: Found only warnings, ${message}`)
       } else if (messages.length > 0) {
-        console.log("Found only messages, passing those to review.")
+        console.log("Danger: Passed, found only messages.")
+      } else if (!messages.length && !fails.length && !messages.length && !warnings.length) {
+        console.log("Danger: Passed review, received no feedback.")
       }
     }
   }