From b2d6e15d48ba63f3cdc21270d01de5d0188bcb35 Mon Sep 17 00:00:00 2001 From: Haacked Date: Mon, 28 Sep 2015 00:32:04 -0700 Subject: [PATCH 1/4] 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. --- Octokit.Tests/Clients/TagsClientTests.cs | 4 +- Octokit/Models/Request/CreateFileRequest.cs | 4 +- Octokit/Models/Request/NewCommit.cs | 4 +- Octokit/Models/Request/NewTag.cs | 2 +- Octokit/Models/Response/Commit.cs | 6 +- Octokit/Models/Response/CommitEntity.cs | 36 ------------ Octokit/Models/Response/Committer.cs | 61 ++++++++++++++++++++ Octokit/Models/Response/GitHubCommit.cs | 6 +- Octokit/Models/Response/GitTag.cs | 4 +- Octokit/Models/Response/PullRequestCommit.cs | 6 +- Octokit/Models/Response/SignatureResponse.cs | 30 ---------- Octokit/Octokit-Mono.csproj | 5 +- Octokit/Octokit-MonoAndroid.csproj | 5 +- Octokit/Octokit-Monotouch.csproj | 5 +- Octokit/Octokit-Portable.csproj | 7 +-- Octokit/Octokit-netcore45.csproj | 5 +- Octokit/Octokit.csproj | 7 +-- ReleaseNotes.md | 1 + 18 files changed, 93 insertions(+), 105 deletions(-) delete mode 100644 Octokit/Models/Response/CommitEntity.cs create mode 100644 Octokit/Models/Response/Committer.cs delete mode 100644 Octokit/Models/Response/SignatureResponse.cs diff --git a/Octokit.Tests/Clients/TagsClientTests.cs b/Octokit.Tests/Clients/TagsClientTests.cs index 7a8ce5f34c..43724514d5 100644 --- a/Octokit.Tests/Clients/TagsClientTests.cs +++ b/Octokit.Tests/Clients/TagsClientTests.cs @@ -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 @@ -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); diff --git a/Octokit/Models/Request/CreateFileRequest.cs b/Octokit/Models/Request/CreateFileRequest.cs index 116d085358..f5a98d398d 100644 --- a/Octokit/Models/Request/CreateFileRequest.cs +++ b/Octokit/Models/Request/CreateFileRequest.cs @@ -34,12 +34,12 @@ protected ContentRequest(string message) /// /// Specifies the committer to use for the commit. This is optional. /// - public SignatureResponse Committer { get; set; } + public Committer Committer { get; set; } /// /// Specifies the author to use for the commit. This is optional. /// - public SignatureResponse Author { get; set; } + public Committer Author { get; set; } } /// diff --git a/Octokit/Models/Request/NewCommit.cs b/Octokit/Models/Request/NewCommit.cs index b4a66858fa..937eb2a4a5 100644 --- a/Octokit/Models/Request/NewCommit.cs +++ b/Octokit/Models/Request/NewCommit.cs @@ -86,7 +86,7 @@ public NewCommit(string message, string tree, string parent) /// /// The author. /// - public SignatureResponse Author { get; set; } + public Committer Author { get; set; } /// /// Gets or sets the person who applied the commit. If omitted, this will be filled in with the @@ -95,7 +95,7 @@ public NewCommit(string message, string tree, string parent) /// /// The committer. /// - public SignatureResponse Committer { get; set; } + public Committer Committer { get; set; } internal string DebuggerDisplay { diff --git a/Octokit/Models/Request/NewTag.cs b/Octokit/Models/Request/NewTag.cs index 96cb24e779..49050a0365 100644 --- a/Octokit/Models/Request/NewTag.cs +++ b/Octokit/Models/Request/NewTag.cs @@ -56,7 +56,7 @@ public class NewTag /// /// The tagger. /// - public SignatureResponse Tagger { get; set; } + public Committer Tagger { get; set; } internal string DebuggerDisplay { diff --git a/Octokit/Models/Response/Commit.cs b/Octokit/Models/Response/Commit.cs index d0b34756ad..374398c0bb 100644 --- a/Octokit/Models/Response/Commit.cs +++ b/Octokit/Models/Response/Commit.cs @@ -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 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 parents, int commentCount) : base(url, label, @ref, sha, user, repository) { Ensure.ArgumentNotNull(parents, "parents"); @@ -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; } diff --git a/Octokit/Models/Response/CommitEntity.cs b/Octokit/Models/Response/CommitEntity.cs deleted file mode 100644 index 5acbecbee5..0000000000 --- a/Octokit/Models/Response/CommitEntity.cs +++ /dev/null @@ -1,36 +0,0 @@ -using System; -using System.Diagnostics; -using System.Globalization; - -namespace Octokit -{ - [DebuggerDisplay("{DebuggerDisplay,nq}")] - public class CommitEntity - { - public CommitEntity() { } - - /// - /// Name of the user - /// - public string Name { get; protected set; } - - /// - /// Email of the user - /// - public string Email { get; protected set; } - - /// - /// Time the commit happened - /// - public DateTime Date { get; protected set; } - - internal string DebuggerDisplay - { - get - { - return String.Format(CultureInfo.InvariantCulture, - "CommitEntity: Name: {0}, Email: {1}, Date: {2}", Name, Email, Date); - } - } - } -} \ No newline at end of file diff --git a/Octokit/Models/Response/Committer.cs b/Octokit/Models/Response/Committer.cs new file mode 100644 index 0000000000..da46505f40 --- /dev/null +++ b/Octokit/Models/Response/Committer.cs @@ -0,0 +1,61 @@ +using System; +using System.Diagnostics; +using System.Globalization; + +namespace Octokit +{ + /// + /// 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. + /// + [DebuggerDisplay("{DebuggerDisplay,nq}")] + public class Committer + { + /// + /// Initializes a new instance of the class. + /// + public Committer() { } + + /// + /// Initializes a new instance of the class. + /// + /// The full name of the author or committer. + /// The email. + /// The date. + public Committer(string name, string email, DateTimeOffset date) + { + Name = name; + Email = email; + Date = date; + } + + /// + /// Gets the name of the author or committer. + /// + /// + /// The name. + /// + public string Name { get; protected set; } + + /// + /// Gets the email of the author or committer. + /// + /// + /// The email. + /// + public string Email { get; protected set; } + + /// + /// Gets the date of the author or contributor's contributions. + /// + /// + /// The date. + /// + public DateTimeOffset Date { get; protected set; } + + internal string DebuggerDisplay + { + get { return String.Format(CultureInfo.InvariantCulture, "Name: {0} Email: {1} Date: {2}", Name, Email, Date); } + } + } +} diff --git a/Octokit/Models/Response/GitHubCommit.cs b/Octokit/Models/Response/GitHubCommit.cs index 556252750d..3f3c679d08 100644 --- a/Octokit/Models/Response/GitHubCommit.cs +++ b/Octokit/Models/Response/GitHubCommit.cs @@ -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 parents, IReadOnlyList 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 parents, IReadOnlyList files) : base(url, label, @ref, sha, user, repository) { Author = author; @@ -24,13 +24,13 @@ public GitHubCommit(string url, string label, string @ref, string sha, User user Files = files; } - public CommitEntity Author { get; protected set; } + public Author Author { get; protected set; } public string CommentsUrl { get; protected set; } public Commit Commit { get; protected set; } - public CommitEntity Committer { get; protected set; } + public Author Committer { get; protected set; } public string HtmlUrl { get; protected set; } diff --git a/Octokit/Models/Response/GitTag.cs b/Octokit/Models/Response/GitTag.cs index 956880d22a..3b0b6d0e9d 100644 --- a/Octokit/Models/Response/GitTag.cs +++ b/Octokit/Models/Response/GitTag.cs @@ -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; @@ -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; } } diff --git a/Octokit/Models/Response/PullRequestCommit.cs b/Octokit/Models/Response/PullRequestCommit.cs index 94c2d330be..0ca38a4973 100644 --- a/Octokit/Models/Response/PullRequestCommit.cs +++ b/Octokit/Models/Response/PullRequestCommit.cs @@ -12,7 +12,7 @@ public class PullRequestCommit { public PullRequestCommit() { } - public PullRequestCommit(SignatureResponse author, Uri commentsUrl, Commit commit, SignatureResponse committer, Uri htmlUrl, IEnumerable parents, string sha, Uri url) + public PullRequestCommit(Committer author, Uri commentsUrl, Commit commit, Committer committer, Uri htmlUrl, IEnumerable parents, string sha, Uri url) { Ensure.ArgumentNotNull(parents, "parents"); @@ -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; } diff --git a/Octokit/Models/Response/SignatureResponse.cs b/Octokit/Models/Response/SignatureResponse.cs deleted file mode 100644 index a56f860990..0000000000 --- a/Octokit/Models/Response/SignatureResponse.cs +++ /dev/null @@ -1,30 +0,0 @@ -using System; -using System.Diagnostics; -using System.Globalization; - -namespace Octokit -{ - [DebuggerDisplay("{DebuggerDisplay,nq}")] - public class SignatureResponse - { - public SignatureResponse() { } - - public SignatureResponse(string name, string email, DateTimeOffset date) - { - Name = name; - Email = email; - Date = date; - } - - public string Name { get; protected set; } - - public string Email { get; protected set; } - - public DateTimeOffset Date { get; protected set; } - - internal string DebuggerDisplay - { - get { return String.Format(CultureInfo.InvariantCulture, "Name: {0} Email: {1} Date: {2}", Name, Email, Date); } - } - } -} diff --git a/Octokit/Octokit-Mono.csproj b/Octokit/Octokit-Mono.csproj index 8b2387cd54..ac6f85e22a 100644 --- a/Octokit/Octokit-Mono.csproj +++ b/Octokit/Octokit-Mono.csproj @@ -379,7 +379,7 @@ - + @@ -403,7 +403,6 @@ - - \ No newline at end of file + diff --git a/Octokit/Octokit-MonoAndroid.csproj b/Octokit/Octokit-MonoAndroid.csproj index 32a51ff5a5..dcf0e36c00 100644 --- a/Octokit/Octokit-MonoAndroid.csproj +++ b/Octokit/Octokit-MonoAndroid.csproj @@ -385,7 +385,7 @@ - + @@ -411,7 +411,6 @@ - - \ No newline at end of file + diff --git a/Octokit/Octokit-Monotouch.csproj b/Octokit/Octokit-Monotouch.csproj index 2c630ce94c..1c66988cae 100644 --- a/Octokit/Octokit-Monotouch.csproj +++ b/Octokit/Octokit-Monotouch.csproj @@ -381,7 +381,7 @@ - + @@ -407,8 +407,7 @@ - - \ No newline at end of file + diff --git a/Octokit/Octokit-Portable.csproj b/Octokit/Octokit-Portable.csproj index 84c3a97fa1..831a637c53 100644 --- a/Octokit/Octokit-Portable.csproj +++ b/Octokit/Octokit-Portable.csproj @@ -375,7 +375,7 @@ - + @@ -400,7 +400,6 @@ - @@ -424,11 +423,11 @@ - - \ No newline at end of file + diff --git a/Octokit/Octokit-netcore45.csproj b/Octokit/Octokit-netcore45.csproj index b9af9386f9..609897848b 100644 --- a/Octokit/Octokit-netcore45.csproj +++ b/Octokit/Octokit-netcore45.csproj @@ -1,5 +1,5 @@  - + Debug @@ -379,7 +379,7 @@ - + @@ -404,7 +404,6 @@ - diff --git a/Octokit/Octokit.csproj b/Octokit/Octokit.csproj index c282bbd36b..f13073ae37 100644 --- a/Octokit/Octokit.csproj +++ b/Octokit/Octokit.csproj @@ -111,7 +111,6 @@ - @@ -311,7 +310,7 @@ - + @@ -435,10 +434,10 @@ - - \ No newline at end of file + diff --git a/ReleaseNotes.md b/ReleaseNotes.md index bc50b59b36..68f95a6802 100644 --- a/ReleaseNotes.md +++ b/ReleaseNotes.md @@ -5,6 +5,7 @@ * Improved: Added `Description` property to `NewTeam` to allow specifying a description for a team - via #915 @haacked * Improved: Added `Description` property to `OrganizationUpdate` to allow specifying a description for an organization - via #915 @haacked * Improved: Added `Before` property to `NotificationsRequest` to find notifications updated before a specific time - via #915 @haacked +* Improved: Renamed `SignatureResponse` to `Committer` and replaced `CommitEntity` with `Committer` - via @haacked * Fixed: Bug that prevented sepecifying a commit message for pull request merges - via #915 @haacked **Breaking Changes:** From 870b34f2be43473f47e8857dd6025367468bff70 Mon Sep 17 00:00:00 2001 From: Haacked Date: Mon, 28 Sep 2015 09:46:28 -0700 Subject: [PATCH 2/4] Add a README for model objects --- Octokit/Models/README.md | 38 ++++++++++++++++++++++++++++++++++++++ Octokit/Octokit.csproj | 1 + 2 files changed, 39 insertions(+) create mode 100644 Octokit/Models/README.md diff --git a/Octokit/Models/README.md b/Octokit/Models/README.md new file mode 100644 index 0000000000..8ce0e1b0d6 --- /dev/null +++ b/Octokit/Models/README.md @@ -0,0 +1,38 @@ +## 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 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. diff --git a/Octokit/Octokit.csproj b/Octokit/Octokit.csproj index f13073ae37..28b068e73e 100644 --- a/Octokit/Octokit.csproj +++ b/Octokit/Octokit.csproj @@ -424,6 +424,7 @@ + Designer From 04f65cc107a52aff953f5dea055edcfd188a23c6 Mon Sep 17 00:00:00 2001 From: Haacked Date: Mon, 28 Sep 2015 09:47:33 -0700 Subject: [PATCH 3/4] Move Committer into Common folder This object is used both in requests and responses. --- Octokit/Models/Common/Committer.cs | 61 ++++++++++++++++++++++++++++++ Octokit/Models/README.md | 3 +- Octokit/Octokit-Mono.csproj | 2 +- Octokit/Octokit-MonoAndroid.csproj | 4 +- Octokit/Octokit-Monotouch.csproj | 2 +- Octokit/Octokit-Portable.csproj | 2 +- Octokit/Octokit-netcore45.csproj | 2 +- Octokit/Octokit.csproj | 2 +- 8 files changed, 70 insertions(+), 8 deletions(-) create mode 100644 Octokit/Models/Common/Committer.cs diff --git a/Octokit/Models/Common/Committer.cs b/Octokit/Models/Common/Committer.cs new file mode 100644 index 0000000000..da46505f40 --- /dev/null +++ b/Octokit/Models/Common/Committer.cs @@ -0,0 +1,61 @@ +using System; +using System.Diagnostics; +using System.Globalization; + +namespace Octokit +{ + /// + /// 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. + /// + [DebuggerDisplay("{DebuggerDisplay,nq}")] + public class Committer + { + /// + /// Initializes a new instance of the class. + /// + public Committer() { } + + /// + /// Initializes a new instance of the class. + /// + /// The full name of the author or committer. + /// The email. + /// The date. + public Committer(string name, string email, DateTimeOffset date) + { + Name = name; + Email = email; + Date = date; + } + + /// + /// Gets the name of the author or committer. + /// + /// + /// The name. + /// + public string Name { get; protected set; } + + /// + /// Gets the email of the author or committer. + /// + /// + /// The email. + /// + public string Email { get; protected set; } + + /// + /// Gets the date of the author or contributor's contributions. + /// + /// + /// The date. + /// + public DateTimeOffset Date { get; protected set; } + + internal string DebuggerDisplay + { + get { return String.Format(CultureInfo.InvariantCulture, "Name: {0} Email: {1} Date: {2}", Name, Email, Date); } + } + } +} diff --git a/Octokit/Models/README.md b/Octokit/Models/README.md index 8ce0e1b0d6..e0c4eee38b 100644 --- a/Octokit/Models/README.md +++ b/Octokit/Models/README.md @@ -25,7 +25,8 @@ 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 need a default constructor as well as one that takes in every parameter. +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. diff --git a/Octokit/Octokit-Mono.csproj b/Octokit/Octokit-Mono.csproj index ac6f85e22a..3d6206fc15 100644 --- a/Octokit/Octokit-Mono.csproj +++ b/Octokit/Octokit-Mono.csproj @@ -379,7 +379,6 @@ - @@ -403,6 +402,7 @@ + diff --git a/Octokit/Octokit-MonoAndroid.csproj b/Octokit/Octokit-MonoAndroid.csproj index dcf0e36c00..ea68c7d3a9 100644 --- a/Octokit/Octokit-MonoAndroid.csproj +++ b/Octokit/Octokit-MonoAndroid.csproj @@ -385,7 +385,6 @@ - @@ -411,6 +410,7 @@ + - + \ No newline at end of file diff --git a/Octokit/Octokit-Monotouch.csproj b/Octokit/Octokit-Monotouch.csproj index 1c66988cae..7b228d6dd9 100644 --- a/Octokit/Octokit-Monotouch.csproj +++ b/Octokit/Octokit-Monotouch.csproj @@ -381,7 +381,6 @@ - @@ -407,6 +406,7 @@ + diff --git a/Octokit/Octokit-Portable.csproj b/Octokit/Octokit-Portable.csproj index 831a637c53..28280e5439 100644 --- a/Octokit/Octokit-Portable.csproj +++ b/Octokit/Octokit-Portable.csproj @@ -375,7 +375,6 @@ - @@ -400,6 +399,7 @@ + diff --git a/Octokit/Octokit-netcore45.csproj b/Octokit/Octokit-netcore45.csproj index 609897848b..6f98071fae 100644 --- a/Octokit/Octokit-netcore45.csproj +++ b/Octokit/Octokit-netcore45.csproj @@ -379,7 +379,6 @@ - @@ -404,6 +403,7 @@ + diff --git a/Octokit/Octokit.csproj b/Octokit/Octokit.csproj index 28b068e73e..3708c95bd4 100644 --- a/Octokit/Octokit.csproj +++ b/Octokit/Octokit.csproj @@ -310,7 +310,7 @@ - + From 8031e4874db385b2122f699930027637dd84f118 Mon Sep 17 00:00:00 2001 From: Haacked Date: Mon, 28 Sep 2015 15:14:54 -0700 Subject: [PATCH 4/4] Add doc comments for Author and Committer --- Octokit/Models/Response/GitHubCommit.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Octokit/Models/Response/GitHubCommit.cs b/Octokit/Models/Response/GitHubCommit.cs index 3f3c679d08..facc929b71 100644 --- a/Octokit/Models/Response/GitHubCommit.cs +++ b/Octokit/Models/Response/GitHubCommit.cs @@ -24,12 +24,22 @@ public GitHubCommit(string url, string label, string @ref, string sha, User user Files = files; } + /// + /// 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. + /// public Author Author { get; protected set; } public string CommentsUrl { get; protected set; } public Commit Commit { get; protected set; } + /// + /// 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. + /// public Author Committer { get; protected set; } public string HtmlUrl { get; protected set; }