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
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
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