Merge pull request #469 from danger/docs

Improve docs #trivial

.travis.yml

@@ -32,7 +32,8 @@ matrix:
     - yarn run link
     - echo "This is the real `danger ci` run on this repo"
     - DEBUG="*" danger ci --verbose
-    - DEBUG="*" danger pr https://github.com/danger/danger-js/pull/465 --verbose # To make sure it isn't broken again in a bad build
+    - echo "Validating that danger pr works as expected"
+    - DEBUG="*" danger pr https://github.com/danger/danger-js/pull/465 --dangerfile dangerfile.circle.js --verbose 
 
   # Create some fake projects at runtime
   - node_js: '7'

docs/guides/the_dangerfile.html.md

@@ -89,21 +89,24 @@ If you'd like to work with some reference material, here are some examples in th
 
 JavaScript:
 
-* **Apps** - [Artsy/metaphysics][meta].
-* **Libraries** - [facebook/react-native][rn], [styled-components/styled-components][sc] and [ReactiveX/rxjs][rxjs].
+* **Libraries** - [facebook/react-native][rn], [facebook/react][r], [styled-components/styled-components][sc] and [ReactiveX/rxjs][rxjs].
+* **Docs** - [bamlab/dev-standards][bamlab]
 
 Some TypeScript examples:
 
 * **Apps** - [Artsy/Emission][emiss]
-* **Libraries** [danger/danger-js][danger-js]
+* **Libraries** [danger/danger-js][danger-js], [apollographql/apollo-client][apollo]
 
 [emiss]: https://github.com/artsy/emission/blob/master/dangerfile.ts
 [danger-js]: https://github.com/danger/danger-js/blob/master/dangerfile.ts
 [meta]: https://github.com/artsy/metaphysics/blob/master/dangerfile.js
 [rn]: https://github.com/facebook/react-native/blob/master/danger/dangerfile.js
+[r]: https://github.com/facebook/react/blob/master/dangerfile.js
 [sc]: https://github.com/styled-components/styled-components/blob/master/dangerfile.js
 [rxjs]: https://github.com/ReactiveX/rxjs/blob/master/dangerfile.js
 [setup]: http://danger.systems/guides/getting_started.html#creating-a-bot-account-for-danger-to-use
 [jest]: https://github.com/facebook/jest
 [transpilation_guide]: /js/tutorials/transiplation.html
 [changelog]: http://danger.systems/js/changelog.html
+[apollo]: https://github.com/apollographql/apollo-client/blob/master/dangerfile.ts
+[bamlab]: https://github.com/bamlab/dev-standards/blob/master/dangerfile.js

docs/tutorials/transpilation.html.md

@@ -16,6 +16,18 @@ A few notes:
 * Babel 7 support for TypeScript is supported
 * Whether you use `dangerfile.ts` or `dangerfile.js` is irrelevant
 
+### TypeScript gotchas
+
+You might have a `src` folder where your actual source code is kept, and adding a `dangerfile.ts` at the root which will break compilation. The answer to this is to add the dangerfile to the `"exclude"` section. Then to get inline errors working correct, add it to the `"include"`. It's a neat little trick. You can see it working in [artsy/emission#tsconfig.json][tsconfig]
+
+```json
+{
+  "compilerOptions": {},
+  "include": ["src/**/*.ts", "src/**/*.tsx", "dangerfile.ts"],
+  "exclude": ["dangerfile.ts", "node_modules"]
+}
+```
+
 ### The "danger" module
 
 The `danger` module is removed before evaluation, it's only there to fake your dev env into working correctly. In reality, all of the exports are added to the global environment. If you import `"danger"` in code that isn't evaluated inside Danger itself, it will raise an exception.
@@ -47,3 +59,5 @@ it("does nothing when there's a PR body", () => {
   })
 })
 ```
+
+[tsconfig]: https://github.com/artsy/emission/blob/master/tsconfig.json

source/commands/danger-pr.ts

@@ -53,7 +53,6 @@ program
   })
 
 setSharedArgs(program).parse(process.argv)
-d(`Starting Danger PR`)
 
 const app = (program as any) as App
 
@@ -78,6 +77,8 @@ if (program.args.length === 0) {
       console.log("Check out: http://danger.systems/js/guides/the_dangerfile.html#working-on-your-dangerfile")
     }
 
+    console.log(`Starting Danger PR on ${pr.repo}#${pr.pullRequestNumber}`)
+
     if (validateDangerfileExists(dangerFile)) {
       d(`executing dangerfile at ${dangerFile}`)
       const source = new FakeCI({ DANGER_TEST_REPO: pr.repo, DANGER_TEST_PR: pr.pullRequestNumber })

source/commands/danger-runner.ts

@@ -31,7 +31,10 @@ program
 
 const argvClone = process.argv.slice(0)
 setSharedArgs(program).parse(argvClone)
-d(`Started Danger Run with ${program.args}`)
+d(`Started Danger runner with ${program.args}`)
+
+console.log(process.env)
+console.log(program.args)
 
 let foundDSL = false
 let runtimeEnv = {} as any

source/commands/danger.ts

@@ -32,9 +32,13 @@ program
     console.log("       http://danger.systems/js/reference.html")
   })
 
+// Commander mutates process.argv
+const originalProcessArgV = process.argv
 program.parse(process.argv)
 
-const showUpgradeNotice = process.env.CI && ['init', 'ci', 'process', 'pr', '--help'].some(cmd => process.argv.includes(cmd))
+const showUpgradeNotice =
+  process.env.CI && ["init", "ci", "process", "pr", "--help"].some(cmd => originalProcessArgV.includes(cmd))
+
 if (showUpgradeNotice) {
   console.error("You may have updated from Danger 2.x -> 3.x without updating from `danger` to `danger ci`.")
 }

source/commands/utils/reporting.ts

@@ -14,3 +14,12 @@ export const resultsWithFailure = (failure: string, moreMarkdown?: string): Dang
     markdowns: moreMarkdown ? [moreMarkdown] : [],
   }
 }
+
+export const mergeResults = (left: DangerResults, right: DangerResults): DangerResults => {
+  return {
+    warnings: [...left.warnings, ...right.warnings],
+    messages: [...left.messages, ...right.messages],
+    fails: [...left.fails, ...right.fails],
+    markdowns: [...left.markdowns, ...right.markdowns],
+  }
+}

source/commands/utils/runDangerSubprocess.ts

@@ -1,9 +1,10 @@
 import * as debug from "debug"
+import * as path from "path"
 import { spawn } from "child_process"
 
 import { DangerDSLJSONType, DangerJSON } from "../../dsl/DangerDSL"
 import { Executor } from "../../runner/Executor"
-import { markdownCode, resultsWithFailure } from "./reporting"
+import { markdownCode, resultsWithFailure, mergeResults } from "./reporting"
 
 const d = debug("danger:runDangerSubprocess")
 
@@ -21,11 +22,11 @@ export const prepareDangerDSL = (dangerDSL: DangerDSLJSONType) => {
 const runDangerSubprocess = (subprocessName: string[], dslJSONString: string, exec: Executor) => {
   let processName = subprocessName[0]
   let args = subprocessName
+  let results = {} as any
   args.shift() // mutate and remove the first element
 
-  d("ARGV:", process.argv)
-
-  d(`Running subprocess: ${processName} - ${args}`)
+  const processDisplayName = path.basename(processName)
+  d(`Running subprocess: ${processDisplayName} - ${args}`)
   const child = spawn(processName, args, { env: process.env })
   let allLogs = ""
 
@@ -36,32 +37,35 @@ const runDangerSubprocess = (subprocessName: string[], dslJSONString: string, ex
     data = data.toString()
     const trimmed = data.trim()
     if (trimmed.startsWith("{") && trimmed.endsWith("}") && trimmed.includes("markdowns")) {
-      d("Got JSON results")
-      const results = JSON.parse(trimmed)
-      await exec.handleResults(results)
+      d("Got JSON results from STDOUT")
+      results = JSON.parse(trimmed)
     } else {
-      if (data.trim().length === 0) {
-        console.log(`stdout: ${data}`)
-      }
+      console.log(`${data}`)
       allLogs += data
     }
   })
 
   child.stderr.on("data", data => {
-    if (data.toString().trim().length === 0) {
-      console.log(`stdout: ${data}`)
+    if (data.toString().trim().length !== 0) {
+      console.log(`${data}`)
     }
   })
 
   child.on("close", async code => {
     d(`child process exited with code ${code}`)
     // Submit an error back to the PR
     if (code) {
-      d(`Handling potential fail`)
+      d(`Handling fail from subprocess`)
       process.exitCode = code
-      const results = resultsWithFailure(`${subprocessName}\` failed.`, "### Log\n\n" + markdownCode(allLogs))
-      await exec.handleResults(results)
+
+      const failResults = resultsWithFailure(`${processDisplayName}\` failed.`, "### Log\n\n" + markdownCode(allLogs))
+      if (results) {
+        results = mergeResults(results, failResults)
+      } else {
+        results = failResults
+      }
     }
+    await exec.handleResults(results)
   })
 }
 

source/runner/Executor.ts

@@ -111,8 +111,6 @@ export class Executor {
    */
   async handleResultsPostingToSTDOUT(results: DangerResults) {
     const { fails, warnings, messages, markdowns } = results
-    process.exitCode = fails.length > 0 ? 1 : 0
-
     if (this.options.jsonOnly) {
       // Format for Danger Process
       const results = {
@@ -123,6 +121,7 @@ export class Executor {
       }
       process.stdout.write(JSON.stringify(results, null, 2))
     } else {
+      this.d("Writing to STDOUT:", results)
       // Human-readable format
 
       const table = [
@@ -167,7 +166,7 @@ export class Executor {
     const failureCount = [...fails, ...warnings].length
     const messageCount = [...messages, ...markdowns].length
 
-    this.d(results)
+    this.d("Posting to platform:", results)
 
     const dangerID = this.options.dangerID
     const failed = fails.length > 0
@@ -186,6 +185,7 @@ export class Executor {
         const are = fails.length === 1 ? "is" : "are"
         console.log(`Failing the build, there ${are} ${fails.length} fail${s}.`)
         if (!successPosting) {
+          this.d("Failing the build due to handleResultsPostingToPlatform not successfully setting a commit status")
           process.exitCode = 1
         }
       } else if (warnings.length > 0) {