Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to flow 0.35.0 #2

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .flowconfig
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
[ignore]
distribution
.*/_tests/.*
.*dangerfile.js

[include]

Expand Down
24 changes: 17 additions & 7 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,22 @@

// Add your own contribution below

### 0.6.1

* upgrades to flow 0.35.0 and fixes associated type errors in covariant/invariant interfaces

### 0.6.0

* omits flow requirement for new test files
* adds support for circleci
* defines CISource properties in flow as read-only

### 0.5.0

* `danger.pr` -> `danger.github.pr`, I've also created interfaces for them - orta
* `warn`, `message`, `markdown` are all ported over to DangerJS - orta
* Shows a HTML table for Danger message - orta
* Now offers a Flow-typed definition file, it's not shipped to their repo yet, you can make it by `npm run export-flowtype` - orta
* Now offers a Flow-typed definition file, it's not shipped to their repo yet, you can make it by `npm run export-flowtype` - orta
* Started turning this into a real project by adding tests - orta

### 0.0.5-0.0.10
Expand All @@ -22,15 +32,15 @@

* Danger will post a comment on a GitHub PR with any Fails - orta

### 0.0.2
### 0.0.2

OK, first usable for others version. Only supports GitHub and Travis CI.

You can run by doing:

```sh
danger
```
```

Make sure you set a `DANGER_GITHUB_API_TOKEN` on your CI - [see the Ruby guide](http://danger.systems/guides/getting_started.html#setting-up-danger-to-run-on-your-ci) for that.

Expand All @@ -42,13 +52,13 @@ git
fail(message: string)
```

`pr` _probably_ won't be sticking around for the long run, but if you're using a `0.0.2` release, you should be OK with that. It's the full metadata of the PR, so [this JSON file](https://raw.githubusercontent.com/danger/danger/master/spec/fixtures/github_api/pr_response.json).
`pr` _probably_ won't be sticking around for the long run, but if you're using a `0.0.2` release, you should be OK with that. It's the full metadata of the PR, so [this JSON file](https://raw.githubusercontent.com/danger/danger/master/spec/fixtures/github_api/pr_response.json).
`git` currently has:

```sh
git.modified_file
git.created_files
git.deleted_files
git.deleted_files
```

which are string arrays of files.
Expand All @@ -66,10 +76,10 @@ if (!hasChangelog) {
}
```

That should do ya. I think. This doens't support babel, and I haven't explored using other modules etc, so
That should do ya. I think. This doens't support babel, and I haven't explored using other modules etc, so

./

### 0.0.1

Not usable for others, only stubs of classes etc.
Not usable for others, only stubs of classes etc.
12 changes: 12 additions & 0 deletions circle.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
machine:
node:
version: 6.1

test:
override:
- npm run build
- npm run link
- danger
post:
- npm run flow
- npm run lint
10 changes: 6 additions & 4 deletions dangerfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@ if (!hasChangelog) {
const jsFiles = danger.git.created_files.filter(path => path.endsWith("js"))

// new js files should have `@flow` at the top
const unFlowedFiles = jsFiles.filter(filepath => {
const content = fs.readFileSync(filepath)
return !content.includes("@flow")
})
// but exclude tests from being flow-ey
const unFlowedFiles = jsFiles.filter(path => !path.endsWith("test.js"))
.filter(filepath => {
const content = fs.readFileSync(filepath)
return !content.includes("@flow")
})

if (unFlowedFiles.length > 0) {
warn(`These new JS files do not have Flow enabled: ${unFlowedFiles.join(", ")}`)
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
"scripts": {
"test": "jest",
"testwatch": "jest --watch",
"flow": "flow check --ignore '.*/_tests/.*, dangerfile.js'",
"flow": "flow check",
"lint": "eslint ./source",
"fix": "eslint ./source --fix",
"prepublish": "npm run build",
Expand Down Expand Up @@ -55,7 +55,7 @@
"eslint-plugin-flowtype": "^2.25.0",
"eslint-plugin-promise": "^3.3.0",
"eslint-plugin-standard": "^2.0.0",
"flow-bin": "^0.32.0",
"flow-bin": "^0.35.0",
"jest-cli": "^16.0.0"
},
"dependencies": {
Expand Down
53 changes: 53 additions & 0 deletions source/ci_source/Circle.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// @flow
"use strict"

import type { Env } from "./ci_source"
import { ensureEnvKeysExist, ensureEnvKeysAreInt } from "./ci_source_helpers"
export default class Circle {
env: Env
constructor(env: Env) { this.env = env }

get name(): string { return "Circle CI" }

get isCI(): boolean {
return ensureEnvKeysExist(this.env, ["CIRCLE_BUILD_NUM"])
}

get isPR(): boolean {
if (ensureEnvKeysExist(this.env, ["CI_PULL_REQUEST"])) {
return true
}

const mustHave = ["CIRCLE_CI_API_TOKEN", "CIRCLE_PROJECT_USERNAME", "CIRCLE_PROJECT_REPONAME", "CIRCLE_BUILD_NUM"]
return ensureEnvKeysExist(this.env, mustHave) && ensureEnvKeysAreInt(this.env, ["CIRCLE_PR_NUMBER"])
}

_prParseURL(): {owner?: string, reponame?: string, id?: string} {
const prUrl = this.env.CI_PULL_REQUEST || ""
const splitSlug = prUrl.split("/")
if (splitSlug.length === 7) {
const owner = splitSlug[3]
const reponame = splitSlug[4]
const id = splitSlug[6]
return {owner, reponame, id}
}
return {}
}

get pullRequestID(): string {
if (this.env.CIRCLE_PR_NUMBER) {
return this.env.CIRCLE_PR_NUMBER
} else {
const {id} = this._prParseURL()
return id || ""
}
}

get repoSlug(): string {
const {owner, reponame} = this._prParseURL()
return (owner && reponame) ? `${owner}/${reponame}` : ""
}

get repoURL(): string { return this.env.CIRCLE_REPOSITORY_URL }
get supportedPlatforms(): string[] { return ["github"] }
}
83 changes: 83 additions & 0 deletions source/ci_source/_tests/_circle.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
import Circle from "../Circle"

const correctEnv = {
"CIRCLE_CI_API_TOKEN": "xxx",
"CIRCLE_PROJECT_USERNAME": "circle_org",
"CIRCLE_PROJECT_REPONAME": "someproject",
"CIRCLE_BUILD_NUM": "1501",
"CIRCLE_PR_NUMBER": "800",
"CI_PULL_REQUEST": "https://github.com/artsy/eigen/pull/800"
}

describe(".isCI", () => {
test("validates when all Circle environment vars are set", () => {
const circle = new Circle(correctEnv)
expect(circle.isCI).toBeTruthy()
})

test("does not validate without josh", () => {
const circle = new Circle({})
expect(circle.isCI).toBeFalsy()
})
})

describe(".isPR", () => {
test("validates when all circle environment vars are set", () => {
const circle = new Circle(correctEnv)
expect(circle.isPR).toBeTruthy()
})

test("does not validate outside of circle", () => {
const circle = new Circle({})
expect(circle.isPR).toBeFalsy()
})

const envs = ["CIRCLE_CI_API_TOKEN", "CIRCLE_PROJECT_USERNAME", "CIRCLE_PROJECT_REPONAME", "CIRCLE_BUILD_NUM"]
envs.forEach((key: string) => {
var env = {
"CIRCLE_CI_API_TOKEN": "xxx",
"CIRCLE_PROJECT_USERNAME": "circle_org",
"CIRCLE_PROJECT_REPONAME": "someproject",
"CIRCLE_BUILD_NUM": "1501",
"CIRCLE_PR_NUMBER": "800",
"CI_PULL_REQUEST": "https://github.com/artsy/eigen/pull/800"
}
env[key] = null

test(`does not validate when ${key} is missing`, () => {
const circle = new Circle({})
expect(circle.isPR).toBeFalsy()
})
})

it("needs to have a PR number", () => {
var env = {
"CIRCLE_PR_NUMBER": "asdasd",
"CIRCLE_REPO_SLUG": "artsy/eigen"
}
const circle = new Circle(env)
expect(circle.isPR).toBeFalsy()
})
})

describe(".pullReuestID", () => {
it("pulls it out of the env", () => {
const circle = new Circle({CIRCLE_PR_NUMBER: "800"})
expect(circle.pullRequestID).toEqual("800")
})

it("can derive it from PR Url", () => {
var env = {
"CI_PULL_REQUEST": "https://github.com/artsy/eigen/pull/23"
}
const circle = new Circle(env)
expect(circle.pullRequestID).toEqual("23")
})
})

describe(".repoSlug", () => {
it("derives it from the PR Url", () => {
const circle = new Circle(correctEnv)
expect(circle.repoSlug).toEqual("artsy/eigen")
})
})
23 changes: 14 additions & 9 deletions source/ci_source/ci_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,34 +7,35 @@ export type Env = any;
/** The shape of an object that represents an individual CI */
export interface CISource {
/** The name, mainly for showing errors */
name: string,
+name: string,

/** The hash of environment variables */
env: Env,
+env: Env,

/** Does this validate as being on a particular CI? */
isCI: boolean,
+isCI: boolean,

/** Does this validate as being on a particular PR on a CI? */
isPR: boolean,
+isPR: boolean,

/** What is the reference slug for this environment? */
repoSlug: string,
+repoSlug: string,

/** What platforms can this CI communicate with? */
supportedPlatforms: string[],
+supportedPlatforms: string[],

/** What unique id can be found for the code review platform's PR */
pullRequestID: string,
+pullRequestID: string,

/** What is the URL for the repo */
repoURL: string,
+repoURL: string,

/** What is the project name */
name: string
+name: string
}

import Travis from "./Travis"
import Circle from "./Circle"
import Fake from "./Fake"

/**
Expand All @@ -46,8 +47,12 @@ import Fake from "./Fake"
export function getCISourceForEnv(env: Env) : ?CISource {
// Fake is what I'm using during dev for the minute
const travis = new Travis(env)
const circle = new Circle(env)

if (travis.isCI) {
return travis
} else if (circle.isCI) {
return circle
} else {
return new Fake()
}
Expand Down
12 changes: 6 additions & 6 deletions source/platforms/platform.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,17 +34,17 @@ export interface Platform {
/** Used internally for getting PR/Repo metadata */
ciSource: CISource;
/** Pulls in the Code Review Metadata for inspection */
getReviewInfo: () => Promise<any>;
+getReviewInfo: () => Promise<any>;
/** Pulls in the Code Review Diff, and offers a succinct user-API for it */
getReviewDiff: () => Promise<GitDSL>;
+getReviewDiff: () => Promise<GitDSL>;
/** Creates a comment on the PR */
createComment: (body: string) => Promise<any>;
+createComment: (body: string) => Promise<any>;
/** Delete the main Danger comment */
deleteMainComment: () => Promise<bool>;
+deleteMainComment: () => Promise<bool>;
/** Replace the main Danger comment */
editMainComment: (newComment: string) => Promise<any>;
+editMainComment: (newComment: string) => Promise<any>;
/** Replace the main Danger comment */
updateOrCreateComment: (newComment: string) => Promise<any>;
+updateOrCreateComment: (newComment: string) => Promise<any>;
}

// /** Download all the comments in a PR */
Expand Down
Loading