Skip to content

Commit

Permalink
Merge pull request #916 from octokit/consolidate-committer-info
Browse files Browse the repository at this point in the history
Consolidate committer info
  • Loading branch information
haacked committed Sep 29, 2015
2 parents 1cea52e + 8031e48 commit 5811d9d
Show file tree
Hide file tree
Showing 20 changed files with 203 additions and 104 deletions.
4 changes: 1 addition & 3 deletions Octokit.Tests/Clients/TagsClientTests.cs
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
using System;
using System.Collections.Generic;
using System.Threading.Tasks;
using NSubstitute;
using Octokit;
using Octokit.Internal;
using Octokit.Tests.Helpers;
using Xunit;

public class TagsClientTests
Expand Down Expand Up @@ -83,7 +81,7 @@ public void PerformsNewTagSerialization()
Tag = "tag-name",
Object = "tag-object",
Type = TaggedType.Tree,
Tagger = new SignatureResponse("tagger-name", "tagger-email", DateTimeOffset.Parse("2013-09-03T13:42:52Z"))
Tagger = new Committer("tagger-name", "tagger-email", DateTimeOffset.Parse("2013-09-03T13:42:52Z"))
};

var json = new SimpleJsonSerializer().Serialize(tag);
Expand Down
61 changes: 61 additions & 0 deletions Octokit/Models/Common/Committer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
using System;
using System.Diagnostics;
using System.Globalization;

namespace Octokit
{
/// <summary>
/// Represents the author or committer to a Git commit. This is the information stored in Git and should not be
/// confused with GitHub account information.
/// </summary>
[DebuggerDisplay("{DebuggerDisplay,nq}")]
public class Committer
{
/// <summary>
/// Initializes a new instance of the <see cref="Committer"/> class.
/// </summary>
public Committer() { }

/// <summary>
/// Initializes a new instance of the <see cref="Committer"/> class.
/// </summary>
/// <param name="name">The full name of the author or committer.</param>
/// <param name="email">The email.</param>
/// <param name="date">The date.</param>
public Committer(string name, string email, DateTimeOffset date)
{
Name = name;
Email = email;
Date = date;
}

/// <summary>
/// Gets the name of the author or committer.
/// </summary>
/// <value>
/// The name.
/// </value>
public string Name { get; protected set; }

/// <summary>
/// Gets the email of the author or committer.
/// </summary>
/// <value>
/// The email.
/// </value>
public string Email { get; protected set; }

/// <summary>
/// Gets the date of the author or contributor's contributions.
/// </summary>
/// <value>
/// The date.
/// </value>
public DateTimeOffset Date { get; protected set; }

internal string DebuggerDisplay
{
get { return String.Format(CultureInfo.InvariantCulture, "Name: {0} Email: {1} Date: {2}", Name, Email, Date); }
}
}
}
39 changes: 39 additions & 0 deletions Octokit/Models/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
## Octokit Models

These either represent the body of a GitHub API request or response.

Request objects should be placed in the, you guessed it, _Request_ folder. Likewise Response objects should be placed
in the _Response_ folder.

Some models can be used for both requests and responses.

### Request models

The general design principle for request models are:

1. They represent the body of a request.
2. Properties that are _required_ by the API should be required by the model and passed in via the constructor.
3. Required porperties should not have a setter since they will be set by the constructor.
4. All other properties should have both a getter and setter.

Note that Octokit.net automatically converts property name to the Ruby casing required by the GitHub API. Thus a
property named `BreakingBad` would be passed as `breaking_bad`.

### Response models

The general design principle for response models are:

1. They should be immutable. As such, properties have a `public` getter and `protected` setter.
2. We want the properties to be read only, but also make it possible to mock the response from API methods.
3. They must be easily serialized and deserialized.
4. They need a default constructor as well as one that takes in every parameter.

We're in the process of reconsidering this design as it's created a few problems for us. Namely that creating these
objects in a unit test is a royal pain.

### Notes

There's a lot of confusion caused by the fact that the GitHub API returns GitHub resources as well as Git resources.
For example, you can use the [Git Data API](https://developer.github.com/v3/git/) to directly manipulate Git objects
such as a `commit`. At the same time, GitHub also has its own `commit` (represented by `GitHubCommit` in Octokit.net)
that contains the GitHub information around the commit such as comments.
4 changes: 2 additions & 2 deletions Octokit/Models/Request/CreateFileRequest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,12 @@ protected ContentRequest(string message)
/// <summary>
/// Specifies the committer to use for the commit. This is optional.
/// </summary>
public SignatureResponse Committer { get; set; }
public Committer Committer { get; set; }

/// <summary>
/// Specifies the author to use for the commit. This is optional.
/// </summary>
public SignatureResponse Author { get; set; }
public Committer Author { get; set; }
}

/// <summary>
Expand Down
4 changes: 2 additions & 2 deletions Octokit/Models/Request/NewCommit.cs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ public NewCommit(string message, string tree, string parent)
/// <value>
/// The author.
/// </value>
public SignatureResponse Author { get; set; }
public Committer Author { get; set; }

/// <summary>
/// Gets or sets the person who applied the commit. If omitted, this will be filled in with the
Expand All @@ -95,7 +95,7 @@ public NewCommit(string message, string tree, string parent)
/// <value>
/// The committer.
/// </value>
public SignatureResponse Committer { get; set; }
public Committer Committer { get; set; }

internal string DebuggerDisplay
{
Expand Down
2 changes: 1 addition & 1 deletion Octokit/Models/Request/NewTag.cs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public class NewTag
/// <value>
/// The tagger.
/// </value>
public SignatureResponse Tagger { get; set; }
public Committer Tagger { get; set; }

internal string DebuggerDisplay
{
Expand Down
6 changes: 3 additions & 3 deletions Octokit/Models/Response/Commit.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ public class Commit : GitReference
{
public Commit() { }

public Commit(string url, string label, string @ref, string sha, User user, Repository repository, string message, SignatureResponse author, SignatureResponse committer, GitReference tree, IEnumerable<GitReference> parents, int commentCount)
public Commit(string url, string label, string @ref, string sha, User user, Repository repository, string message, Committer author, Committer committer, GitReference tree, IEnumerable<GitReference> parents, int commentCount)
: base(url, label, @ref, sha, user, repository)
{
Ensure.ArgumentNotNull(parents, "parents");
Expand All @@ -25,9 +25,9 @@ public Commit(string url, string label, string @ref, string sha, User user, Repo

public string Message { get; protected set; }

public SignatureResponse Author { get; protected set; }
public Committer Author { get; protected set; }

public SignatureResponse Committer { get; protected set; }
public Committer Committer { get; protected set; }

public GitReference Tree { get; protected set; }

Expand Down
36 changes: 0 additions & 36 deletions Octokit/Models/Response/CommitEntity.cs

This file was deleted.

61 changes: 61 additions & 0 deletions Octokit/Models/Response/Committer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
using System;
using System.Diagnostics;
using System.Globalization;

namespace Octokit
{
/// <summary>
/// Represents the author or committer to a Git commit. This is the information stored in Git and should not be
/// confused with GitHub account information.
/// </summary>
[DebuggerDisplay("{DebuggerDisplay,nq}")]
public class Committer
{
/// <summary>
/// Initializes a new instance of the <see cref="Committer"/> class.
/// </summary>
public Committer() { }

/// <summary>
/// Initializes a new instance of the <see cref="Committer"/> class.
/// </summary>
/// <param name="name">The full name of the author or committer.</param>
/// <param name="email">The email.</param>
/// <param name="date">The date.</param>
public Committer(string name, string email, DateTimeOffset date)
{
Name = name;
Email = email;
Date = date;
}

/// <summary>
/// Gets the name of the author or committer.
/// </summary>
/// <value>
/// The name.
/// </value>
public string Name { get; protected set; }

/// <summary>
/// Gets the email of the author or committer.
/// </summary>
/// <value>
/// The email.
/// </value>
public string Email { get; protected set; }

/// <summary>
/// Gets the date of the author or contributor's contributions.
/// </summary>
/// <value>
/// The date.
/// </value>
public DateTimeOffset Date { get; protected set; }

internal string DebuggerDisplay
{
get { return String.Format(CultureInfo.InvariantCulture, "Name: {0} Email: {1} Date: {2}", Name, Email, Date); }
}
}
}
16 changes: 13 additions & 3 deletions Octokit/Models/Response/GitHubCommit.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ public class GitHubCommit : GitReference
{
public GitHubCommit() { }

public GitHubCommit(string url, string label, string @ref, string sha, User user, Repository repository, CommitEntity author, string commentsUrl, Commit commit, CommitEntity committer, string htmlUrl, GitHubCommitStats stats, IReadOnlyList<GitReference> parents, IReadOnlyList<GitHubCommitFile> files)
public GitHubCommit(string url, string label, string @ref, string sha, User user, Repository repository, Author author, string commentsUrl, Commit commit, Author committer, string htmlUrl, GitHubCommitStats stats, IReadOnlyList<GitReference> parents, IReadOnlyList<GitHubCommitFile> files)
: base(url, label, @ref, sha, user, repository)
{
Author = author;
Expand All @@ -24,13 +24,23 @@ public GitHubCommit(string url, string label, string @ref, string sha, User user
Files = files;
}

public CommitEntity Author { get; protected set; }
/// <summary>
/// Gets the GitHub account information for the commit author. It attempts to match the email
/// address used in the commit with the email addresses registered with the GitHub account.
/// If no account corresponds to the commit email, then this property is null.
/// </summary>
public Author Author { get; protected set; }

public string CommentsUrl { get; protected set; }

public Commit Commit { get; protected set; }

public CommitEntity Committer { get; protected set; }
/// <summary>
/// Gets the GitHub account information for the commit committer. It attempts to match the email
/// address used in the commit with the email addresses registered with the GitHub account.
/// If no account corresponds to the commit email, then this property is null.
/// </summary>
public Author Committer { get; protected set; }

public string HtmlUrl { get; protected set; }

Expand Down
4 changes: 2 additions & 2 deletions Octokit/Models/Response/GitTag.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ public class GitTag : GitReference
{
public GitTag() { }

public GitTag(string url, string label, string @ref, string sha, User user, Repository repository, string tag, string message, SignatureResponse tagger, TagObject objectVar)
public GitTag(string url, string label, string @ref, string sha, User user, Repository repository, string tag, string message, Committer tagger, TagObject objectVar)
: base(url, label, @ref, sha, user, repository)
{
Tag = tag;
Expand All @@ -20,7 +20,7 @@ public GitTag(string url, string label, string @ref, string sha, User user, Repo

public string Message { get; protected set; }

public SignatureResponse Tagger { get; protected set; }
public Committer Tagger { get; protected set; }

public TagObject Object { get; protected set; }
}
Expand Down
6 changes: 3 additions & 3 deletions Octokit/Models/Response/PullRequestCommit.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public class PullRequestCommit
{
public PullRequestCommit() { }

public PullRequestCommit(SignatureResponse author, Uri commentsUrl, Commit commit, SignatureResponse committer, Uri htmlUrl, IEnumerable<GitReference> parents, string sha, Uri url)
public PullRequestCommit(Committer author, Uri commentsUrl, Commit commit, Committer committer, Uri htmlUrl, IEnumerable<GitReference> parents, string sha, Uri url)
{
Ensure.ArgumentNotNull(parents, "parents");

Expand All @@ -26,13 +26,13 @@ public PullRequestCommit(SignatureResponse author, Uri commentsUrl, Commit commi
Url = url;
}

public SignatureResponse Author { get; protected set; }
public Committer Author { get; protected set; }

public Uri CommentsUrl { get; protected set; }

public Commit Commit { get; protected set; }

public SignatureResponse Committer { get; protected set; }
public Committer Committer { get; protected set; }

public Uri HtmlUrl { get; protected set; }

Expand Down
30 changes: 0 additions & 30 deletions Octokit/Models/Response/SignatureResponse.cs

This file was deleted.

Loading

0 comments on commit 5811d9d

Please sign in to comment.