-
-
Notifications
You must be signed in to change notification settings - Fork 364
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Adds some more docs, splits it into guides/tutorials/usage
- Loading branch information
Showing
10 changed files
with
310 additions
and
19 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 first coming up with a really long checklist of rules for Danger, and then trying to introduce them all at-once. The entire team may have agreed on the changes, but slower gradual integration 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 with 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, and Danger is catching them out. | ||
|
||
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. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
--- | ||
title: FAQ | ||
subtitle: Plugin creation | ||
layout: guide | ||
order: 0 | ||
--- | ||
|
||
This ensures that Danger only runs when you have the environment variables set up to run. | ||
|
||
### Can Danger comment inline on an PR? | ||
|
||
Not yet, but there is a lot of discussion on [danger-js#77][77] | ||
|
||
### 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]: | ||
[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/ |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
File renamed without changes.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 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 include 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") | ||
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's a few idea 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 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
--- | ||
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, to a native app or 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 | ||
|
||
On 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 a few characters long. | ||
|
||
```js | ||
if (!danger.pr.body.length < 10) { | ||
fail("This pull request needs an description.") | ||
} | ||
``` | ||
|
||
The rules help establish your cultural baselines. | ||
|
||
|
||
|
||
* Checking test changes for new files | ||
* Reference a GitHub Issue | ||
* Warn about large PRs | ||
* Warn on commit rules | ||
|
||
|
||
[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 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
--- | ||
title: Danger + Node Library | ||
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 library | ||
|
||
* CHANGELOG (checking for lib changes) | ||
* Dependencies | ||
* Release PRs | ||
* Interacting with your source, eg. | ||
|
||
``` js | ||
// Always ensure we name all CI providers in the README. These | ||
// regularly get forgotten on a PR adding a new one. | ||
|
||
import { realProviders } from "./source/ci_source/providers" | ||
import Fake from "./source/ci_source/providers/Fake" | ||
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)}`) | ||
} | ||
``` | ||
|
||
and our `danger.d.ts` checks |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
--- | ||
title: TypeScript + Danger JS | ||
subtitle: Plugin creation | ||
layout: guide_js | ||
order: 0 | ||
--- | ||
|
||
### TypeScript | ||
|
||
Danger is built in TypeScript, so we have great support keeping everything typed. | ||
|
||
|
||
### 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" | ||
} | ||
} | ||
} | ||
``` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
--- | ||
title: Extending Danger | ||
subtitle: Plugin creation | ||
layout: guide_js | ||
order: 0 | ||
--- | ||
|