Merge pull request #218 from danger/more_docs

Adds some more docs, splits it into guides/tutorials/usage
danger · Apr 15, 2017 · 65013d0 · 65013d0
2 parents bcdb0ce + 05054fb
commit 65013d0
Show file tree

Hide file tree

Showing 18 changed files with 535 additions and 28 deletions.
diff --git a/docs/guides/culture.html.md b/docs/guides/culture.html.md
@@ -0,0 +1,33 @@
+---
+title: Cultural Changes
+subtitle: Plugin creation
+layout: guide
+order: 0
+---
+
+### Introducing Danger
+
+It can be easy to try and jump straight from no Dangerfile to a 200 line complex set of cultural rules. We'd advise against introducing a long list of rules for Danger all at once. In our experience, gradual integration works better. The entire team may have agreed on the changes, but slower adoption has worked better for teams new to working with Danger.
+
+At Artsy we've found that first just integrating Danger with a single simple rule (like checking for a CHANGELOG entry) then starting to introduce them piece-meal from different contributors has made it easier to go from "Ah, we shouldn't do that again" to "Oh, we could make a Danger rule for that" to "Here's the PR". 
+
+That is your end goal, making it so that everyone feels like it's easy to add and amend the rules as a project evolves. Making dramatic changes erodes that feeling, making regular small ones improves it.
+
+### Phrasing
+
+One of Danger's greatest features is that it can free individuals up on the team to stop being "the person who always requests more tests" on a PR. By moving a lot of the rote tasks in code review to a machine, you free up some mental space to concentrate on more important things. One of the downsides is that it is impossible to provide the same level of nuance in how you provide feedback.
+
+You should use Danger to provide impartial feedback. Consider how these messages come across:
+
+* You have not added a CHANGELOG entry.
+* There isn't a CHANGELOG entry.
+* No CHANGELOG entry.
+* This PR does not include a CHANGELOG entry.
+
+The first feels like a statement that someone has intentionally done something wrong, and Danger has caught them in the act.
+
+The second aims to feel a like like a testing framework telling you "Test Suites: 104 passed, 2 failures". 
+
+The third if done consistently can work out well. Terse entries can work well when you have a large series of rules, as it feels like a check list to do.
+
+The fourth is what we generally try to aim for, an impartial but polite mention about the state of submitted PR.
diff --git a/docs/guides/faq.html.md b/docs/guides/faq.html.md
@@ -0,0 +1,31 @@
+---
+title: FAQ
+subtitle: Plugin creation
+layout: guide
+order: 0
+---
+
+### Can Danger comment inside a file on an PR?
+
+Not yet, but there is a lot of discussion on [danger-js#77][77].
+
+### Can I use the same Dangerfile across many repos?
+
+Ish, it's currently quite complex to set up, but work is on-going on [Danger/Peril][peril]. This is a hosted version of Danger which does not need to run on CI. Using Peril you can use Dangerfiles to reply to basically any github webhook type.
+
+### I want to help influence Danger's direction
+
+We'd recommend first becoming acquainted with the [VISION.md][] inside Danger, this is the long-term plan. Then there are two ways to start contributing today:
+
+ * Opinions are extra welcome on issues marked as [Open For Discussion][open].
+
+ * Well defined work items like features or fixes are marked as [You Can Do This][you-can-do-this].
+
+We keep comments in the public domain, there is a Slack, but it's very rarely used. If you're interested in joining, you can DM [orta][].
+
+[77]: https://github.com/danger/danger-js/issues/77
+[VISION.md]: https://github.com/danger/danger-js/blob/master/VISION.md
+[open]: https://github.com/danger/danger-js/issues?q=is%3Aissue+is%3Aopen+label%3A%22Open+for+Discussion%22
+[you-can-do-this]: https://github.com/danger/danger-js/issues?q=is%3Aissue+is%3Aopen+label%3A%22You+Can+Do+This%22
+[orta]: https://twitter.com/orta/
+[peril]:  https://github.com/danger/peril
diff --git a/docs/getting_started.html.md → docs/guides/getting_started.html.md b/docs/getting_started.html.md → docs/guides/getting_started.html.md
@@ -33,7 +33,7 @@ To get yourself started, try this as your Dangerfile:
 
 ```js
 import { message, danger } from "danger"
-message("OK, this worked @" + danger.github.pr.user.login)
+message(":tada:, this worked @" + danger.github.pr.user.login)
 ```
 
 Which, once you have your authentication set up, will have danger post a message to your PR with your name.
@@ -79,7 +79,7 @@ You can work with GitHub Enterprise by setting 2 environment variables:
 
 For example:
 
-```hs
+```sh
 DANGER_GITHUB_HOST=git.corp.evilcorp.com
 DANGER_GITHUB_API_BASE_URL=https://git.corp.evilcorp.com/api/v3
 ```
@@ -102,7 +102,8 @@ Danger is built to run as a part of this process, so you will need to have this
 
 You should be able to verify that you have successfully integrated Danger by either re-building your CI or pushing your new commits.
 
-[jest-config]: SBJDSKDJBSDKJBG
+
+[jest-config]: https://facebook.github.io/jest/docs/configuration.html
 [github_bots]: https://twitter.com/sebastiangrail/status/750844399563608065
 [github_token]: https://github.com/settings/tokens/new
 [Yarn]: https://yarnpkg.com
diff --git a/docs/the_dangerfile.html.md → docs/guides/the_dangerfile.html.md b/docs/the_dangerfile.html.md → docs/guides/the_dangerfile.html.md
@@ -9,9 +9,19 @@ order: 1
 
 The Danger JS DSL is fully typed via TypeScript (and eventually Flow again.)  These definitions are shipped with the Danger module. If your text editor supports working with type definitions you will get inline-documentation and auto-completion. Visual Studios Code will do this by default for you.
 
+If you are using Babel in your project, your Dangerfile will use the same transpilation settings. If you're using TypeScript + Jest it will work out of the box too, however, if you don't, you should head over to the [TypeScript guide][ts_guide]
+
 ## Working on your Dangerfile
 
-There are two ways to work and test out your Dangerfile, outside of runnig it on your CI. These both rely on 
+There are two ways to work and test out your Dangerfile, outside of running it on your CI and checking the results. 
+
+These both rely on using the GitHub API locally, so you may hit the GitHub API rate-limit or need to have authenticated request for private repos. In which case you can use an access token to do authenticated requests by exposing a token to Danger.
+
+```sh
+export DANGER_GITHUB_API_TOKEN='xxxxxxxxxx'
+```
+
+Then the danger CLI will use these authenticated API calls.
 
 #### Using `danger pr`
 
@@ -121,3 +131,4 @@ Some TypeScript examples:
 [setup]: http://danger.systems/guides/getting_started.html#creating-a-bot-account-for-danger-to-use
 [jest]: https://github.com/facebook/jest
 [ShellJS]: http://github.com/shelljs/shelljs
+[ts_guide]: DFSDGSDFDgdsgdfgd
diff --git a/docs/troubleshooting.html.md → docs/guides/troubleshooting.html.md b/docs/troubleshooting.html.md → docs/guides/troubleshooting.html.md
@@ -5,32 +5,12 @@ layout: guide
 order: 0
 ---
 
-
-
-
-### TypeScript without Jest
-
-You'll need to take the following steps for danger to evaluate your `dangerfile.ts`:
-
-* Install the `ts-jest` package - `yarn add ts-jest --dev`
-* Add the following `jest` section to your `package.json`
-
-```json
-{
-  "jest": {
-    "transform": {
-      ".(ts|tsx)": "<rootDir>/node_modules/ts-jest/preprocessor.js"
-    }
-  }
-}
-```
-
-### I want to only run Danger for internal branches
+### I only want to run Danger for internal contributors
 
 Let's say you run Danger on the same CI service that deploys your code. If that's open source, you don't want to be letting anyone pull out your private env vars. The work around for this is to not simply call Danger on every test run:
 
 ``` sh
 '[ ! -z $DANGER_GITHUB_API_TOKEN ] && yarn danger || echo "Skipping Danger for External Contributor"'
 ```  
 
-This ensures that Danger only runs when you have the environment variables set up to run.
+This ensures that Danger only runs when you have the environment variables set up to run. This is how Danger works for a lot of the mobile projects work in Artsy.
diff --git a/docs/tutorials/dependencies.html.md b/docs/tutorials/dependencies.html.md
@@ -0,0 +1,126 @@
+---
+title: Danger + Dependencies
+subtitle: Plugin creation
+layout: guide_js
+order: 0
+---
+
+## Before we get started
+
+This guide continues after "[Getting Started][started]" - so you should have seen Danger comment on your PRs.
+
+## Keeping on top of your dependencies
+
+Building pretty-much anything in the node ecosystem involves using using external dependencies. In an ideal situation you want to use as few dependencies as possible, and get the most use out of them. Remember that you are shipping your dependencies too, so you are responsible for them to your end-users.
+
+The numerical scale of dependencies can make it tough to feel like you own your entire stack. So let's try and use Danger to give us more insight into changes related to our dependencies.
+
+## Lockfiles
+
+The simplest rule, which we can evolve, is that any time your `package.json` changes you probably want a change to the [`yarn.lock`][lockfile] or [`shrinkwrap.json`][shrinkwrap] file. Yes, not every change to the `package.json` represents a dependency update but we're starting simple. You start off your `Dangerfile` like this:
+
+```js
+import { danger, fail, warn } from "danger"
+import includes from "lodash.includes"
+
+const hasPackageChanges = includes(danger.git.modified_files, "package.json")
+const hasLockfileChanges = includes(danger.git.modified_files, "yarn.lock")
+if (hasPackageChanges && !hasLockfileChanges) {
+  warn("There are package.json changes with no corresponding lockfile changes")
+}
+```
+
+This uses `lodash`'s `_.includes()` function to see if `danger.git.modified_files` includes the package, but not the lockfile.
+
+### Vetting New Dependencies
+
+This works, and for a while, this is enough. Time passes and you hear about a node module with a [CVE](https://cve.mitre.org) against it, let's call it `"spaced-between"`, you want to ensure it isn't added as a dependency.
+
+There are two aspects that you consider:
+
+* Keeping track of changes to `dependencies` (for noted dependencies)
+* Reading the lockfile for the dependency (for transitive dependencies)
+
+### Keeping track of changes to `dependencies`
+
+We can use `danger.git.JSONDiffForFile` to understand the changes to a JSON file during code review. Note: it returns a promise, so we'll need to use `schedule` to make sure it runs async code correctly.
+
+```js
+const blacklist = "spaced-between"
+
+schedule(async () => {
+  const packageDiff = await danger.git.JSONDiffForFile("package.json")
+
+  if (packageDiff.dependencies) {
+      const newDependencies = packageDiff.dependencies.added
+      if (includes(newDependencies, blacklist)) {
+        fail(`Do not add ${blacklist} to our dependencies, see CVE #23")
+      }
+  }
+})
+```
+
+So for example with a diff of `package.json` where spaced-between is added:
+
+```diff
+{
+  "dependencies": {
+    "commander": "^2.9.0",
+    "debug": "^2.6.0"
++    "spaced-between": "^1.1.1",
+    "typescript": "^2.2.1",
+  },
+}
+```
+
+`JSONDiffForFile` will return an object shaped like this:
+
+```js
+{
+  dependencies {
+    added: ["chalk"],
+    removed: [],
+    after: { commander: "^2.9.0", debug: "^2.6.0",  spaced-between: "^1.1.1",  typescript: "^2.2.1" },
+    before: { commander: "^2.9.0", debug: "^2.6.0", typescript: "^2.2.1" },
+  }
+}
+```
+
+Danger can then look inside the added `keys` for your blacklisted module, and fail the build if it is included.
+
+### Parsing the lockfile
+
+You can trust that this dependency is going to be added directly to your project without it being highlighted in code review, but you can't be sure that any updates to your dependency tree won't bring it in transitively. A transitive dependency is one that comes in as a dependency of a dependency, one which isn't added to `packages.json` but is in `node_modules`. So you're going to look at a simple rule that parses the text of the file for your blacklisted module.
+
+```js
+import fs from "fs"
+import contains from "lodash-contains"
+
+const blacklist = "spaced-between"
+const lockfile = fs.readFileSync("yarn.lock").toString()
+
+if (contains(lockfile, blacklist)) {
+  const message = `${blacklist} was added to our dependencies, see CVE #23`
+  const hint = `To find out what introduced it, use \`yarn why ${blacklist}\`.` 
+  fail(`${message}<br/>${hint}`)
+}
+```
+
+Note the use of `readFileSync`, as Danger is running as a script you'll find it simpler to use the synchronous methods when possible. You could improve the above rule by making danger run `yarn why spaced-between` and outputting the text into the messages. We do this in the [danger repo][danger-why] with `child-process` and `execSync`.
+
+### Building from here
+
+This should give you an idea on how to understand changes to your `node_modules`, from here you can create any rules you want using a mix of `JSONDiffForFile`, `fs.readFileSync` and `child_process.execSync`. Here are a few ideas to get you started:
+
+* Convert the check for the package and lockfile to use `JSONDiffForFile` so that it only warns on `dependencies` or `devDependencies`.
+* Ensure you never add `@types/[module]` to `dependencies` but only into `devDependencies`.
+* When a new dependency is added, use a web-service like [libraries.io][libs] to describe the module inline.
+* [Parse][yarn-parse] the `yarn.lock` file, to say how many transitive dependencies are added on every new dependency.
+* When a dependency is removed, and no other dependencies are added, do a thumbs up 👍.
+
+[started]: /js/guides/asdasdasdas
+[lockfile]: https://yarnpkg.com/lang/en/docs/yarn-lock/
+[shrinkwrap]:  https://docs.npmjs.com/cli/shrinkwrap
+[danger-why]: https://github.com/danger/danger-js/blob/8fba6e7c301ac3459c2b0b93264bff7256efd8da/dangerfile.ts#L49
+[libs]: https://libraries.io
+[yarn-parse]: https://www.npmjs.com/package/parse-yarn-lock
diff --git a/docs/tutorials/node-app.html.md b/docs/tutorials/node-app.html.md
@@ -0,0 +1,89 @@
+---
+title: Danger in a Node App
+subtitle: Plugin creation
+layout: guide_js
+order: 0
+---
+
+## Before we get started
+
+This guide continues after "[Getting Started][started]" - so you should have seen Danger comment on your PRs.
+
+## "Node App"
+
+A node app could cover anything from an API, to a website, a native app or a hardware project. The rules on these projects tend to come from your larger dev team culture. In [Artsy][]] a lot of our rules for applications come from trying to have a similar culture between all projects.
+
+## Assignees
+
+We use [a slack bot][no-slacking] to let people know when they've been assigned to a PR, and so the first rule added to an app is a check that there are assignees to your PR. This is a really simple check:
+
+```js
+import { danger, fail, warn } from "danger"
+
+if (!danger.pr.assignee) {
+  fail("This pull request needs an assignee, and optionally include any reviewers.")
+}
+```
+
+The `danger.pr` object is the JSON provided by GitHub to [represent a pull request][pr]. So here we're pulling out the `assignee` key and validating that anything is inside it.
+
+## PR Messages
+
+In a similar vein, we also want to encourage pull requests as a form of documentation. We can help push people in this direction by not allowing the body of a pull request to be less than a few characters long.
+
+```js
+if (!danger.pr.body.length < 10) {
+  fail("This pull request needs an description.")
+}
+```
+
+This can be expanded to all sorts of checks for example:
+
+* Making sure every PR references an issue, or JIRA ticket.
+* Skipping particular rules based on what someone says inside the message. E.g. "This is a trivial PR."
+
+## Results of CI Processes
+
+Let's assume you're using CI for running tests or linters.
+
+```yaml
+script:
+  - yarn lint
+  - yarn test
+  - yarn danger
+```
+
+If your tool does not have an extra log file output option, you can look at using [`tee`][tee] to copy the text output into a file for later reading ( so you'd change `- yarn lint` to `yarn lint | tee 'linter.log'` )
+
+And here's a really simple check that it contains the word "Failed" and to post the logs into the PR.
+
+```js
+import { danger, markdown } from "danger"
+
+import contains from "lodash-contains"
+import fs from "fs"
+
+const testFile = "tests-output.log"
+const linterOutput = fs.readFileSync(testFile).toString()
+
+if (contains(linterOutput, "Failed")) {
+  const code = "```"
+  markdown(`These changes failed to pass the linter:
+
+${code}
+${linterOutput}
+${code}
+  `)
+}
+```
+
+More mature tools may have a JSON output reporter, so you can parse that file and create your own markdown table.
+
+
+
+
+[started]: /js/guides/asdasdasdas
+[Artsy]: http://artsy.github.io
+[no-slacking]: https://github.com/alloy/no-slacking-on-pull-requests-bot
+[pr]: https://developer.github.com/v3/pulls/#get-a-single-pull-request
+[tee]: http://linux.101hacks.com/unix/tee-command-examples/