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

Exposing a GitHub API to users of Danger #227

Merged
merged 7 commits into from
Apr 20, 2017
Merged
Show file tree
Hide file tree
Changes from 6 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
1 change: 1 addition & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

### Master

* Adds `github.api` more to come - @orta
* [Enhancements to `danger.git.diffForFile()`](https://github.com/danger/danger-js/pull/223) - @namuol

* Removed `diffTypes` second argument in favor of `result.added` and `result.removed`
Expand Down
100 changes: 90 additions & 10 deletions dangerfile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ declare function schedule(promise: () => Promise<any | void>): void

import * as fs from "fs"
import * as child_process from "child_process"
import fetch from "node-fetch"
import { distanceInWords } from "date-fns"

// For some reason we're getting type errors on this includes module?
// Wonder if we could move to the includes function in ES2015?
Expand Down Expand Up @@ -46,27 +48,105 @@ const checkForRelease = (packageDiff) => {

// This is a great candidate for being a Danger plugin.

const checkForNewDependencies = (packageDiff) => {
const checkForNewDependencies = async (packageDiff) => {
if (packageDiff.dependencies) {
if (packageDiff.dependencies.added.length) {
const newDependencies = packageDiff.dependencies.added as string[]
warn(`New dependencies added: ${sentence(newDependencies)}.`)

newDependencies.forEach(dep => {
for (const dep of newDependencies) {
const output = child_process.execSync(`yarn why ${dep} --json`)

// Comes as a series of little JSON messages
const usefulJSONContents = output.toString().split(`{"type":"activityEnd","data":{"id":0}}`).pop() as string
const asJSON = usefulJSONContents.split("}\n{").join("},{")

const whyJSON = JSON.parse(`[${asJSON}]`) as any[]
const messages = whyJSON.filter(msg => typeof msg.data === "string")
markdown(`
## ${dep}

${messages.join("\n\n - ")}
`)
})
const messages = whyJSON.filter(msg => typeof msg.data === "string").map(m => m.data)
const yarnDetails = `
<details>
<summary><code>yarn why ${dep}</code> output</summary>
<p><code><ul><li>${messages.join("</li><li>")}
</li></ul></code></p>
</details>
`
// Grab the NPM metadata
let npmMetadata = ""
const npmResponse = await fetch(`https://registry.npmjs.org/${dep}`)
if (npmResponse.ok) {
const tableDeets = [] as [{ name: string, message: string}]
const npm = await npmResponse.json()

if (npm.time && npm.time.created) {
const distance = distanceInWords(new Date(npm.time.created), new Date)
tableDeets.push ({ name: "Created", message: `${distance} ago` })
}

if (npm.time && npm.time.modified) {
const distance = distanceInWords(new Date(npm.time.modified), new Date)
tableDeets.push ({ name: "Last Updated", message: `${distance} ago` })
}

if (npm.license) {
tableDeets.push({ name: "License", message: npm.license })
} else {
tableDeets.push({ name: "License", message: "<b>NO LICENSE FOUND</b>" })
}

if (npm.maintainers) {
tableDeets.push({ name: "Maintainers", message: npm.maintainers.length })
}

if (npm["dist-tags"] && npm["dist-tags"]["latest"]) {
const currentTag = npm["dist-tags"]["latest"]
const tag = npm.versions[currentTag]
tableDeets.push ({ name: "Releases", message: String(Object.keys(npm.versions).length) })
if (tag.dependencies) {
const deps = Object.keys(tag.dependencies)
const depLinks = deps.map(d => `<a href='http: //npmjs.com/package/${d}'>${d}</a>`)
tableDeets.push ({ name: "Direct Dependencies", message: sentence(depLinks) })
}
}

if (npm.keywords && npm.keywords.length) {
tableDeets.push({ name: "Keywords", message: sentence(npm.keywords) })
}

let readme = "This README is too long to show."
if (npm.readme && npm.readme.length < 10000) {
readme = `
<details>
<summary><code>README</code></summary>

${npm.readme}

</details>
`
}

const homepage = npm.homepage ? npm.homepage : `http: //npmjs.com/package/${dep}`

npmMetadata = `
<h2><a href="${homepage}">${dep}</a></h2>

Author: ${npm.author && npm.author.name ? npm.author.name : "Unknown"}
Description: ${npm.description}
Repo: ${homepage}

<table>
<thead><tr><th></th><th width="100%"></th></tr></thead>
${tableDeets.map(deet => `<tr><td>${deet.name}</td><td>${deet.message}</td></tr>`).join("")}
</table>

${readme}
`
} else {
const errorMessage = await npmResponse.text()
npmMetadata = `Error getting NPM details: ${errorMessage}`
}

markdown(`${npmMetadata} ${yarnDetails}`)
}
}
}
}
Expand Down Expand Up @@ -103,7 +183,7 @@ const checkForTypesInDeps = (packageDiff) => {
schedule(async () => {
const packageDiff = await danger.git.JSONDiffForFile("package.json")
checkForRelease(packageDiff)
checkForNewDependencies(packageDiff)
await checkForNewDependencies(packageDiff)
checkForLockfileDiff(packageDiff)
checkForTypesInDeps(packageDiff)
})
Expand Down
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@
"babel-plugin-transform-regenerator": "^6.22.0",
"babel-preset-es2015": "^6.24.0",
"babel-preset-stage-3": "^6.22.0",
"date-fns": "^1.28.3",
"husky": "^0.13.3",
"in-publish": "^2.0.0",
"jest": "^19.0.2",
Expand All @@ -102,6 +103,7 @@
"chalk": "^1.1.1",
"commander": "^2.9.0",
"debug": "^2.6.0",
"github": "^9.2.0",
"jest-config": "^19.0.2",
"jest-environment-node": "^19.0.2",
"jest-runtime": "^19.0.2",
Expand Down
2 changes: 2 additions & 0 deletions source/danger.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,8 @@ export interface GitDSL {

/** The GitHub metadata for your PR */
export interface GitHubDSL {
/** An authenticated API so you can extend danger's behavior. An instance of the "github" npm module. */
api: GitHub,
/** The issue metadata for a code review session */
issue: GitHubIssue,
/** The PR metadata for a code review session */
Expand Down
3 changes: 3 additions & 0 deletions source/dsl/GitHubDSL.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import { GitCommit } from "./Commit"
import * as GitHub from "github"

// This is `danger.github`

/** The GitHub metadata for your PR */
export interface GitHubDSL {
/** An authenticated API so you can extend danger's behavior. An instance of the "github" npm module. */
api: GitHub,
/** The issue metadata for a code review session */
issue: GitHubIssue,
/** The PR metadata for a code review session */
Expand Down
2 changes: 2 additions & 0 deletions source/platforms/GitHub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,10 @@ export class GitHub {
const commits = await this.api.getPullRequestCommits()
const reviews = await this.api.getReviews()
const requested_reviewers = await this.api.getReviewerRequests()
const externalAPI = this.api.getExternalAPI()

return {
api: externalAPI,
issue,
pr,
commits,
Expand Down
4 changes: 4 additions & 0 deletions source/platforms/_tests/_github.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ jest.mock("../github/GitHubAPI", () => {
const fixtures = await requestWithFixturedJSON("requested_reviewers.json")
return await fixtures()
}

getExternalAPI() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@orta since our tests mock the GitHubAPI file, we need to supply the getExternalAPI() function here for the tests to pass. 👍

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:D

return {}
}
}

return { GitHubAPI }
Expand Down
22 changes: 21 additions & 1 deletion source/platforms/github/GitHubAPI.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { GitHubPRDSL, GitHubUser} from "../../dsl/GitHubDSL"
import * as find from "lodash.find"

import * as node_fetch from "node-fetch"
import * as GitHubNodeAPI from "github"

// The Handle the API specific parts of the github

Expand All @@ -25,6 +26,26 @@ export class GitHubAPI {
this.additionalHeaders = {}
}

/**
* Bit weird, yes, but we want something that can be exposed to an end-user.
* I wouldn't have a problem with moving this to use this API under the hood
* but for now that's just a refactor someone can try.
*/
getExternalAPI(): GitHubNodeAPI {
const baseUrl = process.env["DANGER_GITHUB_API_BASE_URL"] || null
const api = new GitHubNodeAPI({
host: baseUrl,
headers: {
...this.additionalHeaders
}
})

if (this.token) {
api.authenticate({ type: "token", token: this.token })
}
return api
}

/**
* Grabs the contents of an individual file on GitHub
*
Expand Down Expand Up @@ -166,7 +187,6 @@ export class GitHubAPI {

return res.ok ? res.json() : { labels: [] }
}

// API Helpers

private api(path: string, headers: any = {}, body: any = {}, method: string) {
Expand Down
Loading