Skip to content

Conversation

@teamdandelion
Copy link

@teamdandelion teamdandelion commented Feb 27, 2018

Paired with @wchargin

Based on discussion at #29

Copy link
Contributor

@wchargin wchargin left a comment

Choose a reason for hiding this comment

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

LGTM modulo small comments below.

subpayload: PullRequestPayload | IssuePayload | CommentPayload | UserPayload,
};

export type EdgeType = "AUTHOR" | "REFERENCE";
Copy link
Contributor

Choose a reason for hiding this comment

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

As discussed: inclusion and co-inclusion will both fall under the REFERENCE type. That is, an issue REFERENCEs its comments, and the comments also REFERENCE their enclosing issues (resp. PR). We don’t see any reason why this should cause ambiguity, because an issue cannot otherwise reference a comment: it has no body in which to do so.

We’re punting on close/reopen actions for now.

@@ -0,0 +1,45 @@
// @flow

import type {ID, GraphNode, GraphEdge} from "./graph";
Copy link
Contributor

Choose a reason for hiding this comment

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

Unused imports.


import type {ID, GraphNode, GraphEdge} from "./graph";

export const GITHUB_PLUGIN_NANE = "sourcecred/github-beta";
Copy link
Contributor

Choose a reason for hiding this comment

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

s/nane/name

Copy link
Contributor

@wchargin wchargin left a comment

Choose a reason for hiding this comment

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

Thanks!

@teamdandelion teamdandelion merged commit 9878313 into master Feb 27, 2018
@teamdandelion teamdandelion deleted the backend-github-types branch February 27, 2018 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants