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

Consolidate committer info #916

Merged
merged 4 commits into from
Sep 29, 2015
Merged

Consolidate committer info #916

merged 4 commits into from
Sep 29, 2015

Conversation

haacked
Copy link
Contributor

@haacked haacked commented Sep 28, 2015

Replace SignatureResponse and CommitEntity with Committer

A recent PR added CommitEntity but we already had SignatureResponse expressly for this purpose.

So this commit renames SignatureResponse to Committer and removes CommitEntity and replaces it with Committer.

I also had to retarget the NetCore45 app so it builds in VS 2015.

@haacked haacked force-pushed the consolidate-committer-info branch from cc2ff64 to d86f826 Compare September 28, 2015 07:37
A recent PR added CommitEntity but we already had
SignatureResponse expressly for this purpose.

So this commit renames SignatureResponse to Committer
and removes CommitEntity and replaces it with Committer.
@haacked haacked force-pushed the consolidate-committer-info branch from d86f826 to ff832de Compare September 28, 2015 07:38
@haacked
Copy link
Contributor Author

haacked commented Sep 28, 2015

/cc @WillsB wanted to make sure you saw this change.

@haacked haacked force-pushed the consolidate-committer-info branch from ff832de to b2d6e15 Compare September 28, 2015 07:48
@heytherewill
Copy link
Contributor

My bad, didn't know about this one. But why was GitHubCommit changed back to use Author instead of Committer? (that was the initial issue)

@haacked
Copy link
Contributor Author

haacked commented Sep 28, 2015

My bad, didn't know about this one. But why was GitHubCommit changed back to use Author instead of Committer? (that was the initial issue)

Because according to the docs here, the author and committer for a GitHubCommit correspond to a GitHub account (aka Author).

The commit property of GitHubCommit also has author and committer but those correspond to Committer.

I think the confusion stems from the fact that the API returns GitHub objects as well as Git objects and Octokit.net has confused the two in some places.

A Committer is the minimal signature that a Git commit contains {'name', 'email', 'date'}. GitHub will try to associate a committer (and author) of a Git commit to a GitHub account. It can't always do that and now that I think about it, I'm not sure what it responds when it can't.

This object is used both in requests and responses.
@haacked
Copy link
Contributor Author

haacked commented Sep 28, 2015

I commented on the original issue

haacked added a commit that referenced this pull request Sep 29, 2015
@haacked haacked merged commit 5811d9d into master Sep 29, 2015
@haacked haacked deleted the consolidate-committer-info branch September 29, 2015 17:26
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.

2 participants